Closed
Bug 846763
Opened 12 years ago
Closed 12 years ago
Browser Feed Preview is broken due to xbl_scopes.
Categories
(SeaMonkey :: Feed Discovery and Preview, defect)
SeaMonkey
Feed Discovery and Preview
Tracking
(seamonkey2.17 unaffected, seamonkey2.18 fixed, seamonkey2.19 fixed)
RESOLVED
FIXED
seamonkey2.19
Tracking | Status | |
---|---|---|
seamonkey2.17 | --- | unaffected |
seamonkey2.18 | --- | fixed |
seamonkey2.19 | --- | fixed |
People
(Reporter: philip.chee, Assigned: neil)
References
Details
Attachments
(1 file)
(deleted),
patch
|
iannbugzilla
:
review+
philip.chee
:
feedback+
iannbugzilla
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
STR:
1. Set dom.xbl_scopes to true
2. Visit http://www.groklaw.net/backend/GrokLaw.rdf
3. Feed preview UI is messed up.
Error Console messages:
Fri Mar 01 2013 22:53:00
Error: ReferenceError: SubscribeHandler is not defined
Source file: chrome://communicator/content/feeds/subscribe.xml
Line: 35
Fri Mar 01 2013 22:53:00
Error: TypeError: this._feedWriter is null
Source file: chrome://communicator/content/feeds/subscribe.js
Line: 18
Flipping dom.xbl_scopes to false and all is good again.
Changing this line:
http://hg.mozilla.org/comm-central/annotate/bead6b9aeb9f/suite/common/feeds/subscribe.xml#l35
From:
SubscribeHandler.init();
to:
window.wrappedJSObject.SubscribeHandler.init();
Allows this to work with dom.xbl_scopes enabled.
Neil says to CC bholley
Comment 1•12 years ago
|
||
Does SubscribeHandler live in untrusted content?
Assignee | ||
Comment 2•12 years ago
|
||
OK, so this is how things used to work:
When we sniff a feed, we replace the document with about:feeds which is a "safe" about: page i.e. it runs as content. This page then used to create a BrowserFeedWriter object which is an XPConnect global constructor and init that.
Unfortunately the subscription page includes XBL and so we needed to ensure that the BrowserFeedWriter object wasn't initialised until after the XBL was constructed. We therefore called an init method from the XBL constructor.
It appears that our XBL now runs as chrome, is that correct? In which case, could we not rip out all of the BrowserFeedWriter goop and talk directly to the nsIFeedWriter service? In particular, would the nsIFeedWriter JS component be able to safely invoke our XBL methods?
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to comment #2)
> Unfortunately the subscription page includes XBL and so we needed to ensure
> that the BrowserFeedWriter object wasn't initialised until after the XBL was
> constructed. We therefore called an init method from the XBL constructor.
Note that Firefox doesn't bother checking that the XBL has been constructed, so it's possible that it will intermittently fail.
Comment 4•12 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #2)
> It appears that our XBL now runs as chrome, is that correct?
It does not. It runs with nsExpandedPrincipal(contentPrincipal), which means it has slightly elevated privileges with respect to the content (including Xray vision), but can't access data from other origins. Additionally, it can't do privileged stuff like talking to Components.
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Bobby Holley from comment #4)
> It runs with nsExpandedPrincipal(contentPrincipal), which means
> it has slightly elevated privileges with respect to the content (including
> Xray vision), but can't access data from other origins. Additionally, it
> can't do privileged stuff like talking to Components.
In that case I think I should merge SubscribeHandler into subscribe.xml and make it add the load and unload event handlers.
Assignee | ||
Comment 6•12 years ago
|
||
* Supported nsIDOMGlobalObjectConstructor so that I don't have to explicitly init() the BrowserFeedWriter object.
* Moved the event handlers from the <body> to the BrowserFeedWriter object.
* Removed the SubscribeHandler object
* Removed the nsIFeedWriter interface
So all the XBL constructor has to do is to create a new BrowserFeedWriter.
(Firefox doesn't actually create the BrowserFeedWriter from its XBL but I thought Gavin might be interested in the simplification anyway.)
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #720189 -
Flags: review?(iann_bugzilla)
Attachment #720189 -
Flags: feedback?(philip.chee)
Attachment #720189 -
Flags: feedback?(gavin.sharp)
Reporter | ||
Comment 8•12 years ago
|
||
Comment on attachment 720189 [details] [diff] [review]
Proposed patch
looks good f=me
> + // nsIDOMGlobalObjectConstructor
> + constructor: function constructor(aWindow) {
I've been meaning to ask what passes a window to the constructor?
Attachment #720189 -
Flags: feedback?(philip.chee) → feedback+
Comment 10•12 years ago
|
||
(In reply to Philip Chee from comment #8)
> > + constructor: function constructor(aWindow) {
> I've been meaning to ask what passes a window to the constructor?
The code at http://hg.mozilla.org/mozilla-central/annotate/8e68f4d73ec4/dom/base/nsDOMClassInfo.cpp#l4029.
Comment 11•12 years ago
|
||
Comment on attachment 720189 [details] [diff] [review]
Proposed patch
r=me with the relevant changes made to suite/feeds/public/moz.build instead of Makefile.in
Attachment #720189 -
Flags: review?(iann_bugzilla) → review+
Assignee | ||
Comment 12•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Target Milestone: --- → seamonkey2.19
Assignee | ||
Comment 13•12 years ago
|
||
For some reason the file removals got lost when I merged the patch to tip.
Pushed comm-central changeset adf2b7942dfd to address that.
Assignee | ||
Comment 14•12 years ago
|
||
Comment on attachment 720189 [details] [diff] [review]
Proposed patch
[Approval Request Comment]
Regression caused by (bug #): 834697
User impact if declined: Feed preview does not work
Testing completed (on m-c, etc.): Merged to aurora
Risk to taking this patch (and alternatives if risky): Risky alternative is to back out bug 590725 and port Firefox's changes from a bug I can't find instead.
String changes made by this patch: None
Attachment #720189 -
Flags: approval-comm-beta?
Assignee | ||
Updated•12 years ago
|
status-seamonkey2.17:
--- → unaffected
status-seamonkey2.18:
--- → affected
status-seamonkey2.19:
--- → fixed
Attachment #720189 -
Flags: approval-comm-beta? → approval-comm-beta+
Assignee | ||
Comment 15•12 years ago
|
||
Comment 16•12 years ago
|
||
Comment on attachment 720189 [details] [diff] [review]
Proposed patch
(In reply to neil@parkwaycc.co.uk from comment #6)
> (Firefox doesn't actually create the BrowserFeedWriter from its XBL but I
> thought Gavin might be interested in the simplification anyway.)
I filed bug 877834.
Attachment #720189 -
Flags: feedback?(gavin.sharp)
You need to log in
before you can comment on or make changes to this bug.
Description
•