Bug 47997 – [HTML5] HTML5 parser indents list items inside definition lists

• Status: RESOLVED
• Resolution: INVALID
• Component: DOM

[Mihai Parparita]

For the input:

 <dl>
   <dd><li>item 1
   <dd><li>item 2
</dl>

(also in the attachment), item 2 is indented under item 2 in ToT WebKit and Chrome. The DOM that I see there:

<dd>
  <li>
    item 1
    <dd>
      <li>item 2</li>
    </dd>
  </li>
</dd>

The DOM with Safari 5.0.2 (which displays item 1 and 2 as siblings):

<dd>
  <li>item 1</li>
</dd>
<dd>
  <li>item 2</li>
</dd>

The DOM with Firefox 4.0b6 (which also displays them as siblings):

<dl>
  <dd>
    <li>
      item 1
      <dd>
        <li>item 2</li>
      </dd>
    </li>
  </dd>
</dl>

The interesting thing is that the DOM is basically the same in Firefox 4.0 and ToT, but they render the two items at the same indentation level anyway.

[Mihai Parparita]

Created attachment 71314 [details]
Test case

[Mihai Parparita]

Forgot to mention that this was originally reported on the Chrome side as http://crbug.com/59962.

[Adam Barth]

We should fix the rendering / layout to match Firefox given that our parsing already matches.

[Adam Barth]

This only occurs in quirks mode.  We've found the CSS rule in Firefox's quirks.css

[Eric Seidel (no email)]

Created attachment 93247 [details]
speculative fix

[Eric Seidel (no email)]

Created attachment 93253 [details]
Patch

[Adam Barth]

Comment on attachment 93253 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=93253&action=review

> LayoutTests/fast/lists/nested-dd-with-lists.html:4
> +   <dd><li>item 1
> +   <dd><li>item 2

Can you use something besides text so we don't need different image baselines for each platform?

[Eric Seidel (no email)]

I thought some fonts were OK to use on all platforms?  I assume the default one is.

[Eric Seidel (no email)]

Really?  I don't believe you.  The number of needlessly duplicated layout test result images would be insane.

[Adam Barth]

(In reply to comment #9)
> Really?  I don't believe you.  The number of needlessly duplicated layout test result images would be insane.

The number of needlessly duplicated layout test result images is insane.

[Eric Seidel (no email)]

Created attachment 93254 [details]
Patch for landing

[Eric Seidel (no email)]

Created attachment 93255 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 93255 [details]
Patch for landing

Rejecting attachment 93255 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'build-..." exit_code: 2

Last 500 characters of output:
tests/xmlhttprequest ................................................................................................................................................................................
http/tests/xmlhttprequest/web-apps ...............
http/tests/xmlhttprequest/workers ...........
http/tests/xmlviewer .
http/tests/xmlviewer/dumpAsText ...........
748.93s total testing time

23506 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
16 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/8684700

[WebKit Commit Bot]

Created attachment 93267 [details]
Archive of layout-test-results from cr-jail-3

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: cr-jail-3  Port: Mac  Platform: Mac OS X 10.6.7

[Eric Seidel (no email)]

It appears AX code doesn't like me changing the style of these inner dds:

--- /tmp/layout-test-results/platform/mac/accessibility/definition-list-term-expected.txt	2011-05-12 01:02:02.000000000 -0700
+++ /tmp/layout-test-results/platform/mac/accessibility/definition-list-term-actual.txt	2011-05-12 01:02:02.000000000 -0700
@@ -10,11 +10,11 @@
 PASS obj.childAtIndex(0).subrole is "AXSubrole: AXTerm"
 PASS obj.childAtIndex(0).roleDescription is "AXRoleDescription: term"
 PASS obj.childAtIndex(1).role is 'AXRole: AXGroup'
-PASS obj.childAtIndex(1).subrole is "AXSubrole: AXDefinition"
-PASS obj.childAtIndex(1).roleDescription is "AXRoleDescription: definition"
+FAIL obj.childAtIndex(1).subrole should be AXSubrole: AXDefinition. Was AXSubrole: .
+FAIL obj.childAtIndex(1).roleDescription should be AXRoleDescription: definition. Was AXRoleDescription: group.
 PASS obj.childAtIndex(1).role is 'AXRole: AXGroup'
-PASS obj.childAtIndex(1).subrole is "AXSubrole: AXDefinition"
-PASS obj.childAtIndex(1).roleDescription is "AXRoleDescription: definition"
+FAIL obj.childAtIndex(1).subrole should be AXSubrole: AXDefinition. Was AXSubrole: .
+FAIL obj.childAtIndex(1).roleDescription should be AXRoleDescription: definition. Was AXRoleDescription: group.
 PASS successfullyParsed is true
 
 TEST COMPLETE

Chris, do you have any suggestions?

[chris fleizach]

It looks like the render tree changed with your change.

It assumes that there will be a render object that can be identified for the

<ol/dl/ul>

It would appear that accessibility can no longer identify if the element is one of these. It does so in AXObjectCache I believe by looking at the tag name for the associated node with a render object

[Adam Barth]

Does that mean the best path forward is to change how the AX logic finds these elements?

[chris fleizach]

(In reply to comment #17)
> Does that mean the best path forward is to change how the AX logic finds these elements?

I don't know what changed the fact that if you ask for renderer->node()->hasTagName(dlTag) it started returning false...

[Adam Barth]

(In reply to comment #18)
> (In reply to comment #17)
> > Does that mean the best path forward is to change how the AX logic finds these elements?
> 
> I don't know what changed the fact that if you ask for renderer->node()->hasTagName(dlTag) it started returning false...

Eric's theory is that some render objects that used to be blocks are now inlines, which might be affecting how the AX code walks the render tree.

[Adam Barth]

Comment on attachment 93255 [details]
Patch for landing

This patch gives the wrong rendering for this document:

TT
<dd>Hello</dd>

[Eric Seidel (no email)]

Basically this bug boils down to: we're now entering into a new DOM state that the previous parser never let us get to.  The old parser used to auto-close <dd> tags, but the HTML5 one doesn't.  Mozilla has some magic CSS which applies a -moz-margin-start for non-nested dds, and turns that margin into a :before { content "\A "; } in the nested case.  It's not entirely clear to me how that works.  But I expect it makes the spaces able to colapse?  And why those don't get collapsed when they're dd's are outside of dl's isn't clear to me either.

Basically I don't really understand the expected rendering of dd (we don't really have enough testing yet), so I feel unclear how this should be fixed.

[Ahmad Saleem]

Attached test case seems to render same as Chrome Canary 112, Firefox Nightly 112 and STP163 and related Chromium bug (in Comment 02) is marked as "WONTFIX" with the comment 21 as reference.

Do we need to do anything here or we can close this? Thanks!