Closed
Bug 505783
Opened 16 years ago
Closed 16 years ago
Setting dynamically xml:base for XHTML elements has no effect in HTML documents
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
People
(Reporter: nanto, Assigned: hsivonen)
References
Details
(Keywords: html5)
Attachments
(3 files)
395 bytes,
text/html
|
Details | |
1.04 KB,
patch
|
sicking
:
review+
sicking
:
approval1.9.2+
|
Details | Diff | Splinter Review |
1.77 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
Setting the xml:base attribute on an XML (including XHTML) element changes the base URI of the element, but Mozilla now ignores xml:base if the elements is an XHTML element and the document is an HTML document (served as text/html).
See the attachment to reproduce this.
Expected:
http://example.org/
http://example.org/foo
Actual:
https://bugzilla.mozilla.org/attachment.cgi?id=nnn
https://bugzilla.mozilla.org/foo
This is a regression. Firefox 3.5 and older works as expected. This breaks resolving-relative-URIs techniques [1] and some Greasemonkey scripts that use the techniques [2] [3].
[1] http://www.google.com/codesearch?q=createElementNS+setAttributeNS+xml%3Abase+file%3A\.js%24
[2] http://userscripts.org/scripts/show/8551
[3] http://userscripts.org/scripts/show/22702
I suppose we could enable xml:base in HTML too :( I'm not a big fan of xml:base in general since I think it's somewhat over complicated, but if people really use it that seems like a reasonable solution.
Assignee | ||
Comment 3•16 years ago
|
||
IIRC, HTML5/Hixie says that script-inserted xml:base in HTML documents should work.
Assignee: nobody → hsivonen
![]() |
||
Comment 4•16 years ago
|
||
Isn't this just a matter of this code in nsGenericHTMLElement::GetBaseURI() bailing out early?
Do we need to make sure to fix this in 1.9.2, if this is breaking real things out there?
We could just take out the early-bail and see what happens, perf-wise, I suppose...
Maybe we should flag nodes that have an ancestor with xml:base set, so that we can fast-path all the other (common!) cases?
Assignee | ||
Comment 5•16 years ago
|
||
Need to get some perf numbers for this.
Assignee | ||
Updated•16 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•16 years ago
|
||
Comment on attachment 394261 [details] [diff] [review]
Don't make xml:base special in HTML
Looks OK on tryserver talos.
Attachment #394261 -
Flags: review?(jonas)
Assignee | ||
Updated•16 years ago
|
Flags: wanted1.9.2?
Attachment #394261 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 7•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/50c82c606fcd
Are the flags on this bug now right for 3.6 approval consideration?
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Is this really important enough for 1.9.2? Seems very much like an edgecase. If you think we need it you need to set the approval 1.9.2? flag on the patch.
Assignee | ||
Comment 9•16 years ago
|
||
Comment on attachment 394261 [details] [diff] [review]
Don't make xml:base special in HTML
This is unlikely to matter for Web compat. However, not having this patch in 1.9.2 could break Greasemonkey scripts or extensions as stated in the bug description.
Attachment #394261 -
Flags: approval1.9.2?
Comment 10•16 years ago
|
||
Comment on attachment 394261 [details] [diff] [review]
Don't make xml:base special in HTML
I can't imagine taking this on branch without a test (and it should have a test anyway, no?)
Assignee | ||
Comment 11•16 years ago
|
||
Could this bug get branch approval with this test case, please?
Attachment #404235 -
Flags: review?(jonas)
Assignee | ||
Updated•16 years ago
|
Flags: in-testsuite?
Comment on attachment 404235 [details] [diff] [review]
Mochitest
Could you *also* test that things work if the <a> is inserted into the document.
r=me with that
Attachment #404235 -
Flags: review?(jonas) → review+
Comment on attachment 394261 [details] [diff] [review]
Don't make xml:base special in HTML
a=sicking with the testcase once fixed per above comment.
Attachment #394261 -
Flags: approval1.9.2? → approval1.9.2+
Assignee | ||
Comment 14•16 years ago
|
||
Test pushed with additional in-document check:
http://hg.mozilla.org/mozilla-central/rev/1470fa9c9f01
Pushed to branch:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/aa636984cd78
Thanks!
status1.9.2:
--- → beta1-fixed
Flags: in-testsuite? → in-testsuite+
Assignee | ||
Updated•15 years ago
|
Flags: wanted1.9.2?
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•