Closed
Bug 850730
Opened 12 years ago
Closed 12 years ago
Firefox UI sets random attributes on root nodes, leads to spec violations
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 22
Tracking | Status | |
---|---|---|
firefox20 | --- | unaffected |
firefox21 | + | fixed |
firefox22 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: fryn)
References
Details
Attachments
(1 file)
1.36 KB,
patch
|
Gavin
:
review+
bajaj
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Bug 790934 added a spec violation that's web-observable. Specifically, it's changing the DOM for about:blank documents, which is web-visible and changing it is a spec violation.
Comment 1•12 years ago
|
||
We should just exclude about:blank (mea culpa: bug 790934 comment 15).
Comment 2•12 years ago
|
||
I was not aware of this about:blank specification, my assumption to exclude about:blank was for optimization reasons.
Many thanks for pointing it out.
I will add the interdiff to exclude about:blank here, hopefully by end of today.
Assignee: nobody → mak77
Updated•12 years ago
|
![]() |
Assignee | |
Comment 3•12 years ago
|
||
(Sorry for the collision, Marco. I wrote this before you assigned it to yourself.)
We need to exclude everything of the pattern:
/^about:blank($|[#?])/i
If we want to avoid using a RegExp, we can just approximate it with the following, which is what this patch does:
doc.documentURI.startsWith("about:") &&
!doc.documentURI.toLowerCase().startsWith("about:blank")
Attachment #724480 -
Flags: review?(gavin.sharp)
Comment 4•12 years ago
|
||
/^about:(?!blank$)/i should be enough as a regex.. though it's probably more readable a simple doc.documentURI.toLowerCase() != "about:blank"
Updated•12 years ago
|
Assignee: mak77 → fyan
Status: NEW → ASSIGNED
Updated•12 years ago
|
Comment 5•12 years ago
|
||
Comment on attachment 724480 [details] [diff] [review]
patch
Given that it's quite unlikely for about:blankFOO to exist, and if it did we wouldn't care about not adding the listeners to it, this seems fine.
Attachment #724480 -
Flags: review?(gavin.sharp) → review+
![]() |
Assignee | |
Comment 6•12 years ago
|
||
Thank you for the quick review, Gavin. :)
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdc2a6bf577b
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → Firefox 22
Version: unspecified → Trunk
![]() |
Reporter | |
Comment 7•12 years ago
|
||
Probably worth backporting to aurora too...
![]() |
Assignee | |
Comment 8•12 years ago
|
||
Comment on attachment 724480 [details] [diff] [review]
patch
[Approval Request Comment]
Bug caused by: the landing of bug 90934
User impact if declined: Firefox would continue changing the DOM of about:blank pages in a way that violates a web spec and is visible to any web page that opens about:blank pages
Testing completed: locally and just landed on mozilla-inbound
Risk to taking this patch: minimal; super localized fix
String or UUID changes made by this patch: none
Attachment #724480 -
Flags: approval-mozilla-aurora?
Comment 9•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Attachment #724480 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
![]() |
Assignee | |
Comment 10•12 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #4)
> /^about:(?!blank$)/i should be enough as a regex..
No, that's not enough, because any web page can open URIs like about:BLAnk?hi that are still interpreted to be the same document as about:blank, and we should not, from our TabsProgressListener, modify the DOM of any document that web pages can create and read.
Comment 11•12 years ago
|
||
status-firefox22:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•