Closed Bug 884795 Opened 6 years ago Closed 6 years ago

DOM built incorrectly for deeply nested <code> elements with the same class name.

Categories

(Core :: HTML: Parser, defect)

21 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: sam.scott, Assigned: wchen)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.116 Safari/537.36

Steps to reproduce:

DOM is not being built correctly for deeply nested <code> elements with the same class name. Here's a web page that demonstrates the bug - the final text node in the outermost <code> element appears as a child of the <p> element (a sibling of what should be its parent <code> element). If you remove the innermost code elements or change their class names to something else (e.g. "new2") the DOM gets built correctly. This same behavior can be reproduced on Opera, Firefox, Chrome, and IE. But Safari builds the DOM correctly.

<!--
To change this template, choose Tools | Templates
and open the template in the editor.
-->
<!DOCTYPE html>
<html>
    <head>
        <title></title>
        <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
        <style type="text/css">
            code.first {
                margin-left:20px;
                margin-top:10pt;
                margin-bottom:10pt;
                font-size: inherit;
                display:block;
            }
            code.new {
                margin-left:30px;
                display:block;
            }
        </style>
    </head>
    <body>
        <p>The JavaScript code below is from the |ajaxTime.html| example.  Take a moment to read it over and try to understand it.
            <code class='first'>
                $(document).ready( function() {
                <code class='new'>
                    var launchAjax = function() {
                    <code class='new'>
                        $.get(
                        <code class='new'>
                            "servers/timeServer.php", 
                            <br>{ 
                            <code class='new'>
                                timezone: $("[name=timezone]").val(), 
                                <br>request: $("[name=type]").val(), 
                                <br>delay: $("[name=delay]").val(), 
                            </code>
                            },
                            <br>function(data, textStatus, jqXHR) {
                            <code class='new'>
                                $("#result").html(data);
                            </code>
                            };
                        </code>
                        )};
                    </code>
                    $("#ajaxButton").click(launchAjax);
                </code>
                }); 
            </code>
        </p>
		<script type="text/javascript">
			alert(document.querySelector(".first").lastChild.data);
		</script>
    </body>
</html>

Here's another page that also shows the bug. If you remove the attribute and value from the outermost code element, the bug goes away:

<code bug="true">
               a
                <code>
                   b
                    <code>
                        c
                        <code>
                            d
                            <code>
                               	e
                            </code>
                           		f
                            <code>
                               	g
                            </code>
                           h
                        </code>
                      i
                    </code>
                   j
                </code>
                k 
            </code>
This is per spec, see the outer loop counter at in the Adoption Agency Algorithm: <http://www.whatwg.org/html/#adoptionAgency>.
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Component: DOM → HTML: Parser
Resolution: --- → INVALID
Wait, why is the adoption agency algorithm involved here at all?  I thought that only deal with tables stuff?
Or rather with misnested stuff, but this is not misnested, right?
Hrm. Trying Henri, then.
Flags: needinfo?(hsivonen)
I think if you're going to mark this as resolved, I need a bit more information. The code was nested properly, so why does the policy for misnested tags apply here?
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Chrome and IE 10 behaving exactly like Firefox here, so this looks like a spec bug and off like a bug in how Firefox follows the spec.

Confirming in the sense that we should follow up by reporting a spec bug and then see how it gets resolved.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(hsivonen)
Testcase: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/2446

I really don't understand what's going on here.
This is related to the Noah's Arc clause:

   http://www.whatwg.org/html/#push-onto-the-list-of-active-formatting-elements

What happens is:

   <code a>    <!-- A, list is A -->
     <code>    <!-- B, list is A,B -->
      <code>   <!-- C, list is A,B,C -->
       <code>  <!-- D, list is A,B,C,D -->
        <code> <!-- E; B gets removed from the list so list is A,C,D,E -->
        </code> <!-- closes E, list is A,C,D -->
       </code> <!-- closes D, lis is A,C -->
      </code> <!-- closes C, list is just A -->
     </code> <!-- closes A, because the list takes precedence over the stack -->

I think the solution is to make the AAA check the stack first and only do its stuff if the current node isn't a matching formatting element.

Filed spec bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=22926
QA Contact: wchen
Assignee: nobody → wchen
QA Contact: wchen
Filed spec bug to make the fix behave a bit better: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24833
Comment on attachment 8386424 [details] [diff] [review]
HTML parser AAA algorithm should only pop when the current node is not in the list of active formatting elements.

http://www.whatwg.org/specs/web-apps/current-work/multipage/tree-construction.html#adoption-agency-algorithm

Implemented the new step 1 of the AAA. Also added some comments and cleaned up existing comments that were strangely formatted.
Attachment #8386424 - Flags: review?(hsivonen)
Please do let me know how this goes. I'd be fascinated to hear your implementation experience.
Comment on attachment 8386424 [details] [diff] [review]
HTML parser AAA algorithm should only pop when the current node is not in the list of active formatting elements.

s/Arc/Ark/ in the comment.

r+ with that.

Thanks for figuring this out.
Attachment #8386424 - Flags: review?(hsivonen) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac15b6e719be
Flags: in-testsuite+
OS: Windows 7 → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/ac15b6e719be
Status: NEW → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.