Skip to content

Commit

Permalink
Implement Element#innerText to conform the spec
Browse files Browse the repository at this point in the history
NOT READY FOR COMMIT
To commit this patch, I need to do:
 - Rebaseline 9500+ layout files for linux, mac, win, since existing
 implementation doesn't conform the spec[1]
 - Fix DOM distiller bug[3]
  It depends on textContent(true) to have newline for <br>
  chromium/dom-distiller#10
  WebTextTest.testGenerateOutputBRElements should have spec complaint test
  expectation
 - Fix CrSettingsSiteDetailsPermissionTest.All
   change expectations to have <option>s


This patch implements Element#innerText to conform the spec[1].
Pass rate of WPT is changed from 78 failures to 6 failures for 213 test cases.

The design doc is https://goo.gl/VW9xxe.

The differences of current implementations are:
 - No more leading/training newlines
 - No more trailing whitespaces
 - At most two newlines between sequences of <p> and <div>.
 - Contents of <select>, <optgroup> and <option> in result.
 - No newline for <br> for disconnected element.

Note: Handling of <select>, <optgroup> and <option> aren't conformed with the
spec[1] since the spec[1] requires to implement Element#innerText specific
CSS handling, ::first-line, ::first-letter, text-transform etc, for contents
of <option>. I filed the issue[2].


[1] https://html.spec.whatwg.org/multipage/dom.html#the-innertext-idl-attribute
[2] whatwg/html#3797 innerText for <select>, <optgroup> and <option>

TBR=alexmos@chromium.org
TBR=dpapad@chromium.org
TBR=dmazzoni@chromium.org
TBR=skyostil@chromium.org

Bug: 651764, 859410
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I48a02db0347d8ebd189f3ef608b31a4a93d89e84
Reviewed-on: https://chromium-review.googlesource.com/1114673
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585756}
  • Loading branch information
yosinch authored and Commit Bot committed Aug 24, 2018
1 parent 40f1a75 commit 72d438e
Show file tree
Hide file tree
Showing 39 changed files with 972 additions and 259 deletions.
8 changes: 4 additions & 4 deletions chrome/browser/chrome_security_exploit_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,9 @@ IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest,

EXPECT_TRUE(content::ExecuteScriptAndExtractString(rfh, script, &body));
EXPECT_EQ(
"\nYour file was not found\n"
"Your file was not found\n"
"It may have been moved or deleted.\n"
"ERR_FILE_NOT_FOUND\n",
"ERR_FILE_NOT_FOUND",
body);
}

Expand Down Expand Up @@ -473,9 +473,9 @@ IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest,

EXPECT_TRUE(content::ExecuteScriptAndExtractString(rfh, script, &body));
EXPECT_EQ(
"\nYour file was not found\n"
"Your file was not found\n"
"It may have been moved or deleted.\n"
"ERR_FILE_NOT_FOUND\n",
"ERR_FILE_NOT_FOUND",
body);
}

Expand Down
31 changes: 22 additions & 9 deletions chrome/test/data/webui/settings/site_details_permission_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ suite('SiteDetailsPermission', function() {
source: testSource,
};
assertEquals(
permissionSourcesNoSetting[testSource],
permissionSourcesNoSetting[testSource] +
(permissionSourcesNoSetting[testSource].length === 0 ?
'Block (default)\nAllow\nBlock\nAsk' :
'\nBlock (default)\nAllow\nBlock\nAsk'),
testElement.$.permissionItem.innerText.trim());
assertEquals(
permissionSourcesNoSetting[testSource] != '',
Expand Down Expand Up @@ -199,7 +202,8 @@ suite('SiteDetailsPermission', function() {
source: settings.SiteSettingSource.EXTENSION,
};
assertEquals(
extensionSourceStrings[testSetting],
extensionSourceStrings[testSetting] +
'\nBlock (default)\nAllow\nBlock\nAsk',
testElement.$.permissionItem.innerText.trim());
assertTrue(testElement.$.permissionItem.classList.contains('two-line'));
assertTrue(testElement.$.permission.disabled);
Expand All @@ -223,7 +227,8 @@ suite('SiteDetailsPermission', function() {
source: settings.SiteSettingSource.POLICY,
};
assertEquals(
policySourceStrings[testSetting],
policySourceStrings[testSetting] +
'\nBlock (default)\nAllow\nBlock\nAsk',
testElement.$.permissionItem.innerText.trim());
assertTrue(testElement.$.permissionItem.classList.contains('two-line'));
assertTrue(testElement.$.permission.disabled);
Expand All @@ -238,7 +243,9 @@ suite('SiteDetailsPermission', function() {
setting: settings.ContentSetting.ASK,
source: settings.SiteSettingSource.DEFAULT,
};
assertEquals('', testElement.$.permissionItem.innerText.trim());
assertEquals(
'Ask (default)\nAllow\nBlock\nAsk',
testElement.$.permissionItem.innerText.trim());
assertFalse(testElement.$.permissionItem.classList.contains('two-line'));
assertFalse(testElement.$.permission.disabled);
});
Expand All @@ -254,7 +261,8 @@ suite('SiteDetailsPermission', function() {
source: settings.SiteSettingSource.DRM_DISABLED,
};
assertEquals(
'To change this setting, first turn on identifiers',
'To change this setting, first turn on identifiers' +
'\nAllow\nBlock\nAsk',
testElement.$.permissionItem.innerText.trim());
assertTrue(testElement.$.permissionItem.classList.contains('two-line'));
assertTrue(testElement.$.permission.disabled);
Expand All @@ -270,7 +278,8 @@ suite('SiteDetailsPermission', function() {
source: settings.SiteSettingSource.ADS_FILTER_BLACKLIST,
};
assertEquals(
'Site tends to show intrusive ads',
'Site tends to show intrusive ads' +
'\nAllow\nBlock\nAsk',
testElement.$.permissionItem.innerText.trim());
assertTrue(testElement.$.permissionItem.classList.contains('two-line'));
assertFalse(testElement.$.permission.disabled);
Expand All @@ -283,7 +292,8 @@ suite('SiteDetailsPermission', function() {
source: settings.SiteSettingSource.PREFERENCE,
};
assertEquals(
'Block if site tends to show intrusive ads',
'Block if site tends to show intrusive ads' +
'\nAllow\nBlock\nAsk',
testElement.$.permissionItem.innerText.trim());
assertTrue(testElement.$.permissionItem.classList.contains('two-line'));
assertFalse(testElement.$.permission.disabled);
Expand All @@ -296,7 +306,8 @@ suite('SiteDetailsPermission', function() {
source: settings.SiteSettingSource.DEFAULT,
};
assertEquals(
'Block if site tends to show intrusive ads',
'Block if site tends to show intrusive ads' +
'\nBlock (default)\nAllow\nBlock\nAsk',
testElement.$.permissionItem.innerText.trim());
assertTrue(testElement.$.permissionItem.classList.contains('two-line'));
assertFalse(testElement.$.permission.disabled);
Expand All @@ -308,7 +319,9 @@ suite('SiteDetailsPermission', function() {
setting: settings.ContentSetting.ALLOW,
source: settings.SiteSettingSource.PREFERENCE,
};
assertEquals('', testElement.$.permissionItem.innerText.trim());
assertEquals(
'Block (default)\nAllow\nBlock\nAsk',
testElement.$.permissionItem.innerText.trim());
assertFalse(testElement.$.permissionItem.classList.contains('two-line'));
assertFalse(testElement.$.permission.disabled);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
AXWebArea AXRoleDescription='HTML content'
++AXTextArea AXRoleDescription='text entry area' AXValue='TextBox1<newline>'
++AXTextArea AXRoleDescription='text entry area' AXValue='TextBox1'
++++AXHeading AXRoleDescription='heading' AXTitle='TextBox1' AXValue='1'
++++++AXStaticText AXRoleDescription='text' AXValue='TextBox1'
++AXTextArea AXRoleDescription='text entry area' AXValue='TextBox2<newline>Some text.'
++AXTextArea AXRoleDescription='text entry area' AXValue='TextBox2<newline><newline>Some text.'
++++AXHeading AXRoleDescription='heading' AXTitle='TextBox2' AXValue='2'
++++++AXStaticText AXRoleDescription='text' AXValue='TextBox2'
++++AXGroup AXRoleDescription='group'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE ia2_hypertext='<obj0><obj1>' n_characters=2 caret_offset=0 n_selections=0
++ROLE_SYSTEM_TEXT value='TextBox1<newline>' FOCUSABLE IA2_STATE_SINGLE_LINE xml-roles:textbox ia2_hypertext='<obj0>' n_characters=1 caret_offset=0 n_selections=0
++ROLE_SYSTEM_TEXT value='TextBox1' FOCUSABLE IA2_STATE_SINGLE_LINE xml-roles:textbox ia2_hypertext='<obj0>' n_characters=1 caret_offset=0 n_selections=0
++++IA2_ROLE_HEADING name='TextBox1' xml-roles:heading ia2_hypertext='TextBox1' n_characters=8 caret_offset=0 n_selections=0
++++++ROLE_SYSTEM_STATICTEXT name='TextBox1' ia2_hypertext='TextBox1' n_characters=8 caret_offset=0 n_selections=0
++ROLE_SYSTEM_TEXT value='TextBox2<newline>Some text.' FOCUSABLE IA2_STATE_MULTI_LINE xml-roles:textbox ia2_hypertext='<obj0><obj1>' n_characters=2 n_selections=0
++ROLE_SYSTEM_TEXT value='TextBox2<newline><newline>Some text.' FOCUSABLE IA2_STATE_MULTI_LINE xml-roles:textbox ia2_hypertext='<obj0><obj1>' n_characters=2 n_selections=0
++++IA2_ROLE_HEADING name='TextBox2' xml-roles:heading ia2_hypertext='TextBox2' n_characters=8 n_selections=0
++++++ROLE_SYSTEM_STATICTEXT name='TextBox2' ia2_hypertext='TextBox2' n_characters=8 n_selections=0
++++IA2_ROLE_PARAGRAPH ia2_hypertext='Some text.' n_characters=10 n_selections=0
++++++ROLE_SYSTEM_STATICTEXT name='Some text.' ia2_hypertext='Some text.' n_characters=10 n_selections=0
++++++ROLE_SYSTEM_STATICTEXT name='Some text.' ia2_hypertext='Some text.' n_characters=10 n_selections=0
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
rootWebArea
++genericContainer editable multiline richlyEditable value='A contenteditable with a link and an and a .<newline><newline>Always expose editable tables as tables.<newline>Editable list item.<newline>' editableRoot=true
++genericContainer editable multiline richlyEditable value='A contenteditable with a link and an and a.<newline><newline>Always expose editable tables as tables.<newline>Editable list item.' editableRoot=true
++++paragraph editable richlyEditable
++++++staticText editable richlyEditable name='A contenteditable with a '
++++++++inlineTextBox name='A contenteditable with a '
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
AXWebArea
++AXTextArea AXValue='A contenteditable with a link and an and a .
++AXTextArea AXValue='A contenteditable with a link and an and a.
Always expose editable tables as tables.
Editable list item.
'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE ia2_hypertext='<obj0><obj1><obj2>' n_selections=0
++IA2_ROLE_SECTION value='A contenteditable with a link and an and a .<newline><newline>Always expose editable tables as tables.<newline>Editable list item.<newline>' FOCUSABLE IA2_STATE_EDITABLE IA2_STATE_MULTI_LINE ia2_hypertext='<obj0><obj1><obj2>' n_selections=0
++IA2_ROLE_SECTION value='A contenteditable with a link and an and a.<newline><newline>Always expose editable tables as tables.<newline>Editable list item.' FOCUSABLE IA2_STATE_EDITABLE IA2_STATE_MULTI_LINE ia2_hypertext='<obj0><obj1><obj2>' n_selections=0
++++IA2_ROLE_PARAGRAPH IA2_STATE_EDITABLE ia2_hypertext='A contenteditable with a <obj1> and an <obj3> and a <obj5>.' n_selections=0
++++++ROLE_SYSTEM_STATICTEXT name='A contenteditable with a ' IA2_STATE_EDITABLE ia2_hypertext='A contenteditable with a ' n_selections=0
++++++ROLE_SYSTEM_LINK name='link' LINKED IA2_STATE_EDITABLE ia2_hypertext='link' n_selections=0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
rootWebArea
++genericContainer editable multiline richlyEditable value='A contenteditable with a link and an and a .<newline><newline>Always expose editable tables as tables.<newline>Editable list item.' editableRoot=true
++genericContainer editable multiline richlyEditable value='A contenteditable with a link and an and a.<newline><newline>Always expose editable tables as tables.<newline>Editable list item.' editableRoot=true
++++paragraph editable richlyEditable
++++++staticText editable richlyEditable name='A contenteditable with a ' TreeData.textSelStartOffset=0
++++++++inlineTextBox name='A contenteditable with a '
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
AXWebArea
++AXTextArea AXValue='A contenteditable with a link and an and a .
++AXTextArea AXValue='A contenteditable with a link and an and a.
Always expose editable tables as tables.
Editable list item.'
++++AXGroup
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE ia2_hypertext='<obj0>' caret_offset=1 n_selections=1 selection_start=0 selection_end=1
++IA2_ROLE_SECTION value='A contenteditable with a link and an and a .<newline><newline>Always expose editable tables as tables.<newline>Editable list item.' FOCUSABLE IA2_STATE_EDITABLE IA2_STATE_MULTI_LINE ia2_hypertext='<obj0><obj1><obj2>' caret_offset=3 n_selections=1 selection_start=0 selection_end=3
++IA2_ROLE_SECTION value='A contenteditable with a link and an and a.<newline><newline>Always expose editable tables as tables.<newline>Editable list item.' FOCUSABLE IA2_STATE_EDITABLE IA2_STATE_MULTI_LINE ia2_hypertext='<obj0><obj1><obj2>' caret_offset=3 n_selections=1 selection_start=0 selection_end=3
++++IA2_ROLE_PARAGRAPH IA2_STATE_EDITABLE ia2_hypertext='A contenteditable with a <obj1> and an <obj3> and a <obj5>.' n_selections=1 selection_start=0 selection_end=44
++++++ROLE_SYSTEM_STATICTEXT name='A contenteditable with a ' IA2_STATE_EDITABLE ia2_hypertext='A contenteditable with a ' n_selections=1 selection_start=0 selection_end=25
++++++ROLE_SYSTEM_LINK name='link' LINKED IA2_STATE_EDITABLE ia2_hypertext='link' n_selections=1 selection_start=0 selection_end=4
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
rootWebArea
++genericContainer editable multiline richlyEditable value='This is editable.<newline><newline>This is not editable.<newline>But this one is.<newline><newline>So is this one.' editableRoot=true
++genericContainer editable multiline richlyEditable value='This is editable.<newline><newline>This is not editable.<newline><newline><newline>But this one is.<newline><newline>So is this one.' editableRoot=true
++++paragraph editable richlyEditable
++++++staticText editable richlyEditable name='This is editable.'
++++++++inlineTextBox name='This is editable.'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
AXWebArea
++AXTextArea AXValue='This is editable.<newline><newline>This is not editable.<newline>But this one is.<newline><newline>So is this one.'
++AXTextArea AXValue='This is editable.<newline><newline>This is not editable.<newline><newline><newline>But this one is.<newline><newline>So is this one.'
++++AXGroup
++++++AXStaticText AXValue='This is editable.'
++++AXStaticText AXValue='This is not editable.'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE ia2_hypertext='<obj0>' n_selections=0
++IA2_ROLE_SECTION value='This is editable.<newline><newline>This is not editable.<newline>But this one is.<newline><newline>So is this one.' FOCUSABLE IA2_STATE_EDITABLE IA2_STATE_MULTI_LINE ia2_hypertext='<obj0>This is not editable.<newline><obj3><obj4>' n_selections=0
++IA2_ROLE_SECTION value='This is editable.<newline><newline>This is not editable.<newline><newline><newline>But this one is.<newline><newline>So is this one.' FOCUSABLE IA2_STATE_EDITABLE IA2_STATE_MULTI_LINE ia2_hypertext='<obj0>This is not editable.<newline><obj3><obj4>' n_selections=0
++++IA2_ROLE_PARAGRAPH IA2_STATE_EDITABLE ia2_hypertext='This is editable.' n_selections=0
++++++ROLE_SYSTEM_STATICTEXT name='This is editable.' IA2_STATE_EDITABLE ia2_hypertext='This is editable.' n_selections=0
++++ROLE_SYSTEM_STATICTEXT name='This is not editable.' ia2_hypertext='This is not editable.' n_selections=0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@ Tests renderer: in cross origin object.
requested url: http://foo.com/
requested url: http://bar.com/
Blocked a frame with origin "http://foo.com" from accessing a cross-origin frame.

Pass
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ crbug.com/854840 fast/table/backgr_border-table-quirks.html [ Failure ]
# Superscript text off by 1px
crbug.com/636993 external/wpt/css/css-text-decor/text-decoration-color.html [ Failure ]

# text-overflow:ellipsis and paint fragment
crbug.com/873957 http/tests/devtools/network/network-cookies-pane.js [ Failure ]

# rightsizing-grid.html is truly flaky, show flakiness on reload

# Features that do not have active plans to support or turn on.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE

PASS successfullyParsed is true


TEST COMPLETE
Got notification: MenuListValueChanged

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,43 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE

PASS successfullyParsed is true


TEST COMPLETE

PASS accessibleOne.isSelected is true

PASS accessibleOne.isSelectedOptionActive is true

PASS accessibleTwo.isSelected is false

PASS accessibleTwo.isSelectedOptionActive is false

PASS accessibleThree.isSelected is false

PASS accessibleThree.isSelectedOptionActive is false

PASS accessibleOne.isSelected is false

PASS accessibleOne.isSelectedOptionActive is false

PASS accessibleTwo.isSelected is true

PASS accessibleTwo.isSelectedOptionActive is true

PASS accessibleThree.isSelected is false

PASS accessibleThree.isSelectedOptionActive is false

PASS accessibleOne.isSelected is false

PASS accessibleOne.isSelectedOptionActive is false

PASS accessibleTwo.isSelected is true

PASS accessibleTwo.isSelectedOptionActive is false

PASS accessibleThree.isSelected is true

PASS accessibleThree.isSelectedOptionActive is true
List notification: ActiveDescendantChanged
List notification: SelectedChildrenChanged
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
<div id="test2">foo <table style="display:inline-table"><tr><td>2</td></tr></table> bar</div>
<script>
test(() => {
assert_equals('hello 1 world', document.getElementById('test1').innerText,
assert_equals(document.getElementById('test1').innerText, 'hello1world',
'test1.innerText');
assert_equals('foo 2 bar', document.getElementById('test2').innerText,
assert_equals(document.getElementById('test2').innerText, 'foo 2 bar',
'test2.innerText');
}, 'Checks that the text iterator is emitting a space before and after an inline table');
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@
<div id="test4">Lorem<div style="position: absolute;"></div>ipsum</div>
<script>
test(() => assert_equals(
'Lorem-ipsum',
document.getElementById('test1').innerText),
document.getElementById('test1').innerText,
'Lorem\n-\nipsum'),
'1 float:left minus');
test(() => assert_equals(
'Loremipsum',
document.getElementById('test2').innerText),
document.getElementById('test2').innerText,
'Lorem\nipsum'),
'2 float:left empty');
test(() => assert_equals(
'Lorem-ipsum',
document.getElementById('test3').innerText),
document.getElementById('test3').innerText,
'Lorem\n-\nipsum'),
'3 position:absolute minus');
test(() => assert_equals(
'Loremipsum',
document.getElementById('test4').innerText),
document.getElementById('test4').innerText,
'Lorem\nipsum'),
'4 position:absolute empty');
</script>
Loading

0 comments on commit 72d438e

Please sign in to comment.