777632 - prevent pageMods from operating on certain content

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Mark Hammond [:markh] [:mhammond]]

Firefox is landing some "social" work which creates a new sidebar and loads content from a "provider" in that sidebar, and we don't want PageMods to be able to operate on that content.  Ideally there should be a way to express (in XUL or something else) that a particular <browser> element can't be injected with a PageMod.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Alex, can you see if there's any platform stuff we need for this?

Comment 2

[Alexandre Poirot [:ochameau]]

I went through our page-mod source code and haven't been able to spot any (hacky) way to prevent that:
https://github.com/mozilla/addon-sdk/blob/master/packages/addon-kit/lib/page-mod.js
We can easily provide such internal feature, but what is your feeling about existing addons?

Note that we are listening to `document-element-inserted` event and even if addon developer pass "*" as match pattern, it will only match http, https and ftp URIs.
But I'd imagine that social sidebar document is loaded from a http URI?

At the end, I think it would be just better if we add some code in SDK to ensure that the document is a child document from a Tab and reject everything else.
(We are currently applying on document hosted in hidden window if they have http URIs ...)

Comment 3

[Dave Townsend [:mossop]]

So my thoughts here were that when we get 'document-element-inserted' we can use existing APIs (horrible nsIDocShell ones IIRC, but existing nonetheless) to get the <browser> or <iframe> loading the document. Then we could just check if say that element is type="content" or something, or look for an attribute that specifically stops pagemode nopagemod="true". Alex's suggestion of just only applying for tabs makes sense too though.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

The sidebar in question is loaded from a http(s) URL, and is inside a browser with type="content".  There is also another window we'd like to avoid the injection which is hosted in the hidden window.  However, IIUC, in almost all cases for the tab, the <browser> element in question will often have type="content-primary" or type="content-targetable" (not sure of the spelling of the last one).  So it might be possible to skip browsers with plain type="content" (and possibly another exception for those in the hidden window)

Beyond that, I think the idea of restricting the pagemod only to pages in tabs might be the best option as we are likely to see more and more bits of the browser implemented in HTML (eg, we also have a <browser> hosted in a <panel> which I *suspect* is going to be hit by the pagemod, but I've not tested it).  If we went the nopagemod="true" route, my concern is people will add, eg, a new <browser> in a panel and be blissfully unaware of the entire pageMod world, and thus not know they should add that attribute.

Regardless, I completely agree the fix isn't obvious :)

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 519

Erik, Feel free to redirect this review to irakli if you do not feel confident about this review.

Comment 6

[Alexandre Poirot [:ochameau]]

So in this patch I'm cleaning up page-mod a bit by removing old workaround needed for Firefox 4-. And I'm restricting page-mod to only tab document and their iframes.

Comment 7

[Jeff Griffiths (:canuckistani) (:⚡︎)]

+1, Alex - on the face of it this feels like the right boundary for page-mods. In particular, the new UX designs implement sidebars with web content similar to this, and it is hard to see a valid reason for 1 add-on to be able to modify another add-on's sidebar content.

Comment 8

[Alexandre Poirot [:ochameau]]

chrome-mod should be the API to hack with these other things.
It already allows to modify chrome document (which can't be modified by page-mod, even in its current state)

Comment 9

[Erik Vold [:erikvold] (please needinfo? me)]

I like the idea Alex, I just want to confirm the tests pass on Fennec before pulling.

Comment 10

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/b6329768740eb3681dec00f555115defb45473e2
Bug 777632: Apply page-mod only on (child) tab documents. r=@erikvold