Bug 47997 - [HTML5] HTML5 parser indents list items inside definition lists
Summary: [HTML5] HTML5 parser indents list items inside definition lists
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 41115
  Show dependency treegraph
 
Reported: 2010-10-20 12:03 PDT by Mihai Parparita
Modified: 2023-02-18 12:25 PST (History)
7 users (show)

See Also:


Attachments
Test case (47 bytes, text/html)
2010-10-20 12:04 PDT, Mihai Parparita
no flags Details
speculative fix (1.10 KB, patch)
2011-05-11 22:30 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (8.90 KB, patch)
2011-05-11 23:01 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (7.35 KB, patch)
2011-05-11 23:07 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (6.89 KB, patch)
2011-05-11 23:09 PDT, Eric Seidel (no email)
abarth: review-
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from cr-jail-3 (205.90 KB, application/zip)
2011-05-12 01:38 PDT, WebKit Commit Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Parparita 2010-10-20 12:03:57 PDT
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.
Comment 1 Mihai Parparita 2010-10-20 12:04:22 PDT
Created attachment 71314 [details]
Test case
Comment 2 Mihai Parparita 2010-10-20 12:06:21 PDT
Forgot to mention that this was originally reported on the Chrome side as http://crbug.com/59962.
Comment 3 Adam Barth 2011-05-11 22:12:51 PDT
We should fix the rendering / layout to match Firefox given that our parsing already matches.
Comment 4 Adam Barth 2011-05-11 22:22:45 PDT
This only occurs in quirks mode.  We've found the CSS rule in Firefox's quirks.css
Comment 5 Eric Seidel (no email) 2011-05-11 22:30:35 PDT
Created attachment 93247 [details]
speculative fix
Comment 6 Eric Seidel (no email) 2011-05-11 23:01:14 PDT
Created attachment 93253 [details]
Patch
Comment 7 Adam Barth 2011-05-11 23:03:21 PDT
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?
Comment 8 Eric Seidel (no email) 2011-05-11 23:04:16 PDT
I thought some fonts were OK to use on all platforms?  I assume the default one is.
Comment 9 Eric Seidel (no email) 2011-05-11 23:06:06 PDT
Really?  I don't believe you.  The number of needlessly duplicated layout test result images would be insane.
Comment 10 Adam Barth 2011-05-11 23:07:17 PDT
(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.
Comment 11 Eric Seidel (no email) 2011-05-11 23:07:42 PDT
Created attachment 93254 [details]
Patch for landing
Comment 12 Eric Seidel (no email) 2011-05-11 23:09:17 PDT
Created attachment 93255 [details]
Patch for landing
Comment 13 WebKit Commit Bot 2011-05-12 01:38:25 PDT
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
Comment 14 WebKit Commit Bot 2011-05-12 01:38:29 PDT
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
Comment 15 Eric Seidel (no email) 2011-05-12 10:13:06 PDT
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?
Comment 16 chris fleizach 2011-05-12 10:24:30 PDT
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
Comment 17 Adam Barth 2011-05-12 10:56:22 PDT
Does that mean the best path forward is to change how the AX logic finds these elements?
Comment 18 chris fleizach 2011-05-12 11:58:13 PDT
(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...
Comment 19 Adam Barth 2011-05-12 12:01:05 PDT
(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.
Comment 20 Adam Barth 2011-05-12 13:30:41 PDT
Comment on attachment 93255 [details]
Patch for landing

This patch gives the wrong rendering for this document:

TT
<dd>Hello</dd>
Comment 21 Eric Seidel (no email) 2011-05-17 11:03:08 PDT
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.
Comment 22 Ahmad Saleem 2023-02-18 06:37:47 PST
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!