Closed
Bug 673759
(namedspaceOverriding)
Opened 13 years ago
Closed 12 years ago
[MathML3] Remove namedspace value overriding
Categories
(Core :: MathML, defect)
Core
MathML
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: fredw, Assigned: fredw)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 8 obsolete files)
(deleted),
patch
|
karlt
:
review+
emorley
:
checkin+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
karlt
:
review+
|
Details | Diff | Splinter Review |
The REC says:
"MathML2 allowed the binding of namedspaces to new values. It appears that this capability was never implemented, and is now deprecated; namedspaces are now considered constants. For backwards compatibility, the following attributes are accepted on the mstyle element, but are expected to have no effect."
We implement namedspace value overriding in nsMathMLFrame::ParseNamedSpaceValue. Removing this will allow to get rid of many almost never used nsGkAtoms for namespaces. This will also simplify the parsing code: we could move the parsing for namespaces into nsMathMLElement::ParseNumericValue (and add a flag PARSE_ALLOW_NAMEDSPACE).
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → hage.jonathan
Status: NEW → ASSIGNED
Comment 1•13 years ago
|
||
Comment 2•13 years ago
|
||
Comment 3•13 years ago
|
||
Comment 4•13 years ago
|
||
Attachment #549072 -
Attachment is obsolete: true
Attachment #549088 -
Attachment is obsolete: true
Comment 5•13 years ago
|
||
Attachment #550018 -
Attachment is obsolete: true
Assignee | ||
Comment 6•13 years ago
|
||
(In reply to comment #5)
> Created attachment 550043 [details] [diff] [review] [diff] [details] [review]
> Moving nsMathMLFrame::ParseNamedSpaceValue into
> nsMathMLElement::ParseNumericValue + Removing of namedspace value overriding
Compilation fails with this patch:
/src-obj/mozilla/src/content/base/src/nsTreeSanitizer.cpp:952: error: ‘negativemediummathspace_’ is not a member of ‘nsGkAtoms’
...
The nsGkAtoms for spaces should be removed everywhere.
Comment 7•13 years ago
|
||
Attachment #550043 -
Attachment is obsolete: true
Comment 8•13 years ago
|
||
Attachment #551404 -
Attachment is obsolete: true
Comment 9•13 years ago
|
||
Attachment #551407 -
Attachment is obsolete: true
Assignee | ||
Comment 10•13 years ago
|
||
Comment on attachment 549073 [details] [diff] [review]
Reftest for positive namedspaces
We can probably already take this reftest.
Attachment #549073 -
Flags: review?(karlt)
Updated•13 years ago
|
Attachment #549073 -
Flags: review?(karlt) → review+
Assignee | ||
Comment 11•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Whiteboard: [good first bug] → [good first bug][please push attachment 549073]
Comment 12•13 years ago
|
||
I'm presuming this needs to be left open for the other patch? Should a review be set on that patch?
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5ce0b70c2e1
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [good first bug][please push attachment 549073] → [good first bug] [leave open for remaining patch]
Target Milestone: --- → mozilla9
Updated•13 years ago
|
Attachment #549073 -
Flags: checkin+
Comment 13•13 years ago
|
||
Comment on attachment 549073 [details] [diff] [review]
Reftest for positive namedspaces
https://hg.mozilla.org/mozilla-central/rev/f5ce0b70c2e1
Comment 14•13 years ago
|
||
Remaining patch isn't going to make it for mozilla9, so removing target milestone for now.
Target Milestone: mozilla9 → ---
Assignee | ||
Updated•13 years ago
|
Assignee: hage.jonathan → fred.wang
Assignee | ||
Comment 15•13 years ago
|
||
Attachment #551414 -
Attachment is obsolete: true
Assignee | ||
Comment 16•13 years ago
|
||
Part 2, removing the corresponding MathML Atoms. To apply over patch from bug 696647.
I'm not sure this patch is good idea, though. Bug 324472 comment 5 says that atoms are needed for clipboard operations.
Assignee | ||
Comment 17•13 years ago
|
||
Assignee | ||
Comment 18•13 years ago
|
||
Comment on attachment 586105 [details] [diff] [review]
Patch V3 - part 1
Note: I have not changed the flags passed to ParseNumericValue, as I expect this job to be done in bug 411227.
Attachment #586105 -
Flags: review?(karlt)
Assignee | ||
Updated•13 years ago
|
Attachment #586105 -
Flags: review?(karlt)
Assignee | ||
Updated•13 years ago
|
Assignee | ||
Updated•13 years ago
|
Attachment #586107 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Attachment #586105 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Attachment #586107 -
Attachment is obsolete: false
Assignee | ||
Updated•13 years ago
|
Whiteboard: [good first bug] [leave open for remaining patch] → [leave open for remaining patch]
Assignee | ||
Updated•13 years ago
|
Keywords: helpwanted
Assignee | ||
Comment 19•13 years ago
|
||
Comment on attachment 586107 [details] [diff] [review]
Patch V3 - part 2
So now bug 677036 is fixed, it should not be possible to override namedspace. Karl, do you think we should take attachment 586107 [details] [diff] [review]? Bug 324472 comment 5 indicates that atoms are needed for clipboard operations. However, I don't think people have ever used namedspace overriding, so that should not be a problem?
Attachment #586107 -
Flags: review?(karlt)
Comment 20•13 years ago
|
||
Comment on attachment 586107 [details] [diff] [review]
Patch V3 - part 2
Yes, makes sense to remove these now, thanks.
Attachment #586107 -
Flags: review?(karlt) → review+
Assignee | ||
Comment 21•13 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=5adfc5ff68c7
I'm not sure how the build results can be red (with empty summaries) but the tests are executed and seem successful.
Assignee | ||
Comment 22•12 years ago
|
||
(In reply to Frédéric Wang (:fredw) from comment #21)
> https://tbpl.mozilla.org/?tree=Try&rev=5adfc5ff68c7
>
> I'm not sure how the build results can be red (with empty summaries) but the
> tests are executed and seem successful.
Karl, do you have any idea about this problem? I can build opt/debug builds without problems with this patch applied. I tried the patch twice on the Try server and always got this build results in red, with empty summaries.
Comment 23•12 years ago
|
||
I don't know what happened there but some others had similar problems
e.g. https://tbpl.mozilla.org/?tree=Try&rev=be00392bbc1f
I'd try pulling and rebasing against the latest known good changeset.
Assignee | ||
Comment 24•12 years ago
|
||
OK, that looks better:
https://tbpl.mozilla.org/?tree=Try&rev=3dc56ac6f8c3
Keywords: checkin-needed
Whiteboard: [leave open for remaining patch]
Comment 25•12 years ago
|
||
Keywords: checkin-needed
Target Milestone: --- → mozilla15
Comment 26•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•12 years ago
|
Keywords: dev-doc-needed
Comment 27•12 years ago
|
||
Updated docs:
https://developer.mozilla.org/en/MathML/Attributes/Values#Constants
https://developer.mozilla.org/en/Firefox_15_for_developers#MathML
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•