Closed Bug 340494 Opened 14 years ago Closed 5 years ago

access document.domain at about:plugins produces NS_ERROR_FAILURE. No error at about:config

Categories

(Core :: Security, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: bugzilla, Assigned: akshendra521994, Mentored)

References

Details

(Whiteboard: [lang=c++])

Attachments

(1 file, 2 obsolete files)

I'm checking the domain on every page load in my extension and I noticed that when aobut:plugins load I get:

Error: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMNSHTMLDocument.domain]
Source file: chrome://tdccms/content/main.js
Line: 518

it does NOT happen when I load about:config or about:cache. It only seems to happen when I load about:plugins

why? is there something special with about:plugins or is this a bug?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060605 Minefield/3.0a1 ID:2006060505 [cairo]
I don't have a good answer... unlike the others, though, about:plugins is created using document.write(), wonder if that has anything to do with it.
Is this still a problem?
Duplicate of 417021?
Status: NEW → UNCONFIRMED
Ever confirmed: false
Bug 417021 that is. Sorry, did not mean to change status.
Status: UNCONFIRMED → NEW
Ever confirmed: true
about:plugins is not XHTML, so no.

What it is is system-principal, so there is no principal URI to be had.

I guess we could just return "" in this case instead of throwing if someone cares about this case...  That would sort of match the spec.
Mentor: bzbarsky
Whiteboard: [lang=c++]
I will like to work on this. 
Boris, can you hint me where to find to relevant code.
Flags: needinfo?(bzbarsky)
And also is this on all platforms or just WIN?
This is all platforms, and the relevant code is in nsHTMLDocument::GetDomain over in dom/html/nsHTMLDocument.cpp
Flags: needinfo?(bzbarsky)
Attached patch 340494_documentDomain.patch (obsolete) — Splinter Review
Attachment #8519536 - Flags: review?(bzbarsky)
Attachment #8519536 - Flags: review?(bzbarsky)
Akshendra Pratap Singh, why did you remove the review request?  Was there an issue with the patch?
Flags: needinfo?(akshendra521994)
I thought that there could be a better solution then the one I tried in this patch. But since then I didn't got much time to look at that because of my academics. I will submit a new one within this week.
Am sorry if that caused any trouble. :)
Flags: needinfo?(akshendra521994)
No, no trouble, just making sure you're not waiting on me or something.  ;)
Attachment #8519536 - Flags: review?(bzbarsky)
Comment on attachment 8519536 [details] [diff] [review]
340494_documentDomain.patch

r=me
Attachment #8519536 - Flags: review?(bzbarsky) → review+
Do you have the ability to push this to try?
Flags: needinfo?(akshendra521994)
Not yet, but I have applied for that.
Flags: needinfo?(akshendra521994)
https://hg.mozilla.org/mozilla-central/rev/c14fd3305767
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Did this fix bug 417021 as well? Or can the getter still throw?
Yes, it did.  We should remove the Throws annontation from the IDL.
Flags: needinfo?(akshendra521994)
Does that mean I have to remove the use of ErrorResult& rv?
Flags: needinfo?(akshendra521994)
Attached patch 340494_documentDomain.patch (obsolete) — Splinter Review
Attachment #8519536 - Attachment is obsolete: true
Attachment #8530627 - Flags: review?(bzbarsky)
Comment on attachment 8530627 [details] [diff] [review]
340494_documentDomain.patch

No, this is removing GetDomain from nsIDOMHTMLDocument, which is not desirable.

Instead, you can just have the getter be NS_IMETHODIMP and make sure it always returns NS_OK.
Attachment #8530627 - Flags: review?(bzbarsky) → review-
Attachment #8530627 - Attachment is obsolete: true
Attachment #8530647 - Flags: review?(bzbarsky)
Comment on attachment 8530647 [details] [diff] [review]
340494_documentDomain.patch

r=me
Attachment #8530647 - Flags: review?(bzbarsky) → review+
Is there something left to do?
Flags: needinfo?(bzbarsky)
Asking for someone to commit it through this keyword.
Flags: needinfo?(bzbarsky)
Keywords: checkin-needed
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I'm really confused. A patch already landed in this bug and now we're landing another patch that appears to not be built on top of it? Oh, and the previously landed patch is on Aurora now too.

Can we please sort this mess out before landing more under this bug number?
Keywords: checkin-needed
Ryan, the follow up patch is cleanup. I guess it doesn't matter whether that misses a train.
Yeah, the second patch should really have gone into a separate bug, imo....  Akshendra Pratap Singh, would you re-resolving this bug and creating a new one for landing the cleanup?
Flags: needinfo?(akshendra521994)
Actually just re-resolving.  Please put the followup into a separate bug.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Added the cleanup 
Bug 1110789
Flags: needinfo?(akshendra521994)
You need to log in before you can comment on or make changes to this bug.