612529 - Make annotation-xml allow HTML content

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P2)

Description

[Henri Sivonen (:hsivonen)]

The HTML5 spec changed how annotation-xml is handled in the parsing algorithm. Need to implement those changes.

Comment 1

[Henri Sivonen (:hsivonen)]

Per discussion with jst, nominating all HTML5 parsing spec change bugs as
blockers.

This bug should be fixed by Firefox 4 to allow MathML authors use HTML content in their annotations. Deferring the fix would poison the feature for some time as authors would shy away from it. Also, this bug makes Firefox fail tests in the best know HTML5 parsing test suite.

Comment 2

[Johnny Stenback (:jst)]

Yup, I think we should fix this for Firefox 4. Seems very low risk too (should only affect annotations in MathML).

Comment 3

[Henri Sivonen (:hsivonen)]

Attachment: Part 1: Pack element flags into a bitfield

I'll be introducing more flags, so in order to avoid wasting a whole word for each, let's pack this stuff into an int.

The ugliness that is
        this.flags = (scoping ? (elementName.getFlags() | ElementName.SCOPING)
                : (elementName.getFlags() & ~ElementName.SCOPING))
                & ~(ElementName.SPECIAL | ElementName.FOSTER_PARENTING);
will go away in the next parts. It's there just to provide a safe checkpoint that shows that I haven't changed any observable behavior, yet.

Comment 4

[Henri Sivonen (:hsivonen)]

Attachment: Part 2: Rework how scopingness and specialness is handled in stack nodes; refresh the set of scoping elements to spec

Comment 5

[Henri Sivonen (:hsivonen)]

Attachment: Part 3: Make <annotation-xml> an HTML integration point depending on the encoding attribute

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 491794 [details] [diff] [review]
Part 1: Pack element flags into a bitfield

What is the purpose of this? If you want to save memory, you can get the same effect in C++ using bitfields, i.e. something like

PRInt32 group : 7;
PRInt32 special : 1;
PRInt32 scoping : 1;
PRInt32 fosterParenting : 1;

while keeping the code as readable as it was before.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 492658 [details] [diff] [review]
Part 2: Rework how scopingness and specialness is handled in stack nodes; refresh the set of scoping elements to spec

rs=me

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 492659 [details] [diff] [review]
Part 3: Make <annotation-xml> an HTML integration point depending on the encoding attribute

rs=me

Comment 9

[Henri Sivonen (:hsivonen)]

(In reply to comment #6)
> Comment on attachment 491794 [details] [diff] [review]
> Part 1: Pack element flags into a bitfield
> 
> What is the purpose of this?

The purpose is to avoid wasting a word for each flag. I expect to put more flags in element names when implementing the sanitizer.

> If you want to save memory, you can get the same
> effect in C++ using bitfields, i.e. something like
> 
> PRInt32 group : 7;
> PRInt32 special : 1;
> PRInt32 scoping : 1;
> PRInt32 fosterParenting : 1;
> 
> while keeping the code as readable as it was before.

That would compile roughly to the same machine code as the code in the patch. So this is a matter of how nice the C++ code looks. But the C++ code isn't the real source code (in the "preferred form for making modifications" sense). The Java code is. Java doesn't support the syntax you suggest, and adding annotations to enable the translator to emit the C++ syntax you suggest probably wouldn't be a readability win for the Java code and would still waste a word per flag on the Java side.

Also, I think the code with explicit bit masking is still so readable that using the syntactic sugar you propose wouldn't be much of a readability win for the C++ code.

Comment 10

[Henri Sivonen (:hsivonen)]

(In reply to comment #9)
> (In reply to comment #6)
> > Comment on attachment 491794 [details] [diff] [review] [details]
> > Part 1: Pack element flags into a bitfield
> > 
> > What is the purpose of this?
> 
> The purpose is to avoid wasting a word for each flag. I expect to put more
> flags in element names when implementing the sanitizer.

Also, it makes it easier to copy all flags in one operation.

Comment 11

[Henri Sivonen (:hsivonen)]

http://hg.mozilla.org/mozilla-central/rev/941694c1b9c9
http://hg.mozilla.org/mozilla-central/rev/16d8c6a5b502
http://hg.mozilla.org/mozilla-central/rev/944cd0847692