Closed Bug 612529 Opened 9 years ago Closed 9 years ago

Make annotation-xml allow HTML content

Categories

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

defect

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: hsivonen, Assigned: hsivonen)

Details

Attachments

(3 files)

The HTML5 spec changed how annotation-xml is handled in the parsing algorithm. Need to implement those changes.
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.
blocking2.0: --- → ?
Yup, I think we should fix this for Firefox 4. Seems very low risk too (should only affect annotations in MathML).
blocking2.0: ? → betaN+
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.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Attachment #491794 - Flags: review?(jonas)
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 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
Attachment #492658 - Flags: review?(jonas) → review+
Comment on attachment 492659 [details] [diff] [review]
Part 3: Make <annotation-xml> an HTML integration point depending on the encoding attribute

rs=me
Attachment #492659 - Flags: review?(jonas) → review+
(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.
(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.
http://hg.mozilla.org/mozilla-central/rev/941694c1b9c9
http://hg.mozilla.org/mozilla-central/rev/16d8c6a5b502
http://hg.mozilla.org/mozilla-central/rev/944cd0847692
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Priority: -- → P2
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.