Closed Bug 808464 Opened 12 years ago Closed 12 years ago

HTML parser is non-conforming for particular <dd> + <optgroup> case

Categories

(Core :: DOM: HTML Parser, defect)

Product:
Core
Component:
DOM: HTML Parser
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla19

People

(Reporter: sideshowbarker, Assigned: hsivonen)

References

(
URL
)

Details

Attachments

(1 file)

Refresh the list of special elements, remove a bogus scopingness check from <dd> handling
27.86 KB, patch
smaug
: review+
Details | Diff | Splinter Review 
See http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=1892

Per the HTML spec, the following markup:

<!DOCTYPE html><body><dd><optgroup><dd></dd></body>

...should be parsed as:

<!DOCTYPE HTML><html><head></head><body><dd><optgroup></optgroup></dd><dd></dd></body></html>

WebKit, Presto, Trident, and html5lib all correctly parse is that way. But the Gecko parser instead gives this:

<!DOCTYPE HTML><html><head></head><body><dd><optgroup><dd></dd></optgroup></dd></body></html>

Note: I didn't find this case myself; Yasuhiko Minamide and Shunsuke Mori report it in their paper, "Reachability Analysis of the HTML5 Parser Specification and Its Application to Compatibility Testing" http://link.springer.com/chapter/10.1007%2F978-3-642-32759-9_26
optgroup had somehow ended up on the "special" list even though it’s not on the "special" list in the spec today. I guess optgroup has been on the "special" in the spec at some point in the past.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Comment on attachment 680553 [details] [diff] [review]
Refresh the list of special elements, remove a bogus scopingness check from <dd> handling

AFAICT, of the elements whose specialness is changed here, only optgroup’s and menuitem’s specialness is detectable. However, I didn’t add a test for menuitem, because it’s not in the spec.
Attachment #680553 - Flags: review?(bugs)
Comment on attachment 680553 [details] [diff] [review]
Refresh the list of special elements, remove a bogus scopingness check from <dd> handling


>-    public static final ElementName IMAGE = new ElementName("image", "image", TreeBuilder.IMAGE | SPECIAL);
>+    public static final ElementName IMAGE = new ElementName("image", "image", TreeBuilder.IMAGE);
Oh, fun. <img> is special but <image> isn't.

>-                                    } else if (node.isScoping()
>-                                            || (node.isSpecial()
>-                                                    && node.name != "p"
>-                                                    && node.name != "address" && node.name != "div")) {
>+                                    } else if (node.isSpecial()
>+                                            && node.name != "p"
>+                                            && node.name != "address"
>+                                            && node.name != "div") {
Could you explain this change.


Could you explain why only optgroup and menuitem changes are detectable.



r+ assuming I get some explanation :)
Attachment #680553 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #4)
> Comment on attachment 680553 [details] [diff] [review]
> Refresh the list of special elements, remove a bogus scopingness check from
> <dd> handling
> 
> 
> >-    public static final ElementName IMAGE = new ElementName("image", "image", TreeBuilder.IMAGE | SPECIAL);
> >+    public static final ElementName IMAGE = new ElementName("image", "image", TreeBuilder.IMAGE);
> Oh, fun. <img> is special but <image> isn't.

HTML image never ends up on the stack. It is changed into img before it goes on the stack. The specialness is only queried for stuff that’s on the stack.

But more importantly, the current spec does not list HTML "image" as "special".

> >-                                    } else if (node.isScoping()
> >-                                            || (node.isSpecial()
> >-                                                    && node.name != "p"
> >-                                                    && node.name != "address" && node.name != "div")) {
> >+                                    } else if (node.isSpecial()
> >+                                            && node.name != "p"
> >+                                            && node.name != "address"
> >+                                            && node.name != "div") {
> Could you explain this change.

If you look at the spec, there is no "is it scoping?" check there. Maybe there was when I wrote that code, but there isn’t anymore.

> Could you explain why only optgroup and menuitem changes are detectable.

image: see above. Does not go on the stack.
keygen: Empty, so does not go on the stack.
track: Empty, so does not go on the stack.
xmp: The tokenizer makes sure not to emit start tags when in xmp, so xmp cannot have element children, so there’s no opportunity to check if xmp is actually "special".

I made the "special" list match the spec even in the undetectable cases.

> r+ assuming I get some explanation :)

Thanks.
https://hg.mozilla.org/mozilla-central/rev/a137fc5f83c5
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: