Last Comment Bug 777632 - prevent pageMods from operating on certain content
: prevent pageMods from operating on certain content
Status: RESOLVED FIXED
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
P1 normal (vote)
: 1.11
Assigned To: Alexandre Poirot [:ochameau]
:
:
Mentors:
: 765005 (view as bug list)
Depends on:
Blocks: 765005
  Show dependency treegraph
 
Reported: 2012-07-25 23:49 PDT by Mark Hammond [:markh]
Modified: 2012-10-29 22:45 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Pull request 519 (165 bytes, text/html)
2012-08-01 09:09 PDT, Alexandre Poirot [:ochameau]
evold: review+
Details

Description User image Mark Hammond [:markh] 2012-07-25 23:49:07 PDT
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 User image Wes Kocher (:KWierso) 2012-07-26 11:28:53 PDT
Alex, can you see if there's any platform stuff we need for this?
Comment 2 User image Alexandre Poirot [:ochameau] 2012-07-31 08:16:59 PDT
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 User image Dave Townsend [:mossop] 2012-07-31 10:19:11 PDT
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 User image Mark Hammond [:markh] 2012-07-31 16:13:29 PDT
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 User image Alexandre Poirot [:ochameau] 2012-08-01 09:09:51 PDT
Created attachment 647972 [details]
Pull request 519

Erik, Feel free to redirect this review to irakli if you do not feel confident about this review.
Comment 6 User image Alexandre Poirot [:ochameau] 2012-08-01 09:12:08 PDT
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 User image Jeff Griffiths (:canuckistani) (:⚡︎) 2012-08-01 09:31:48 PDT
+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 User image Alexandre Poirot [:ochameau] 2012-08-01 09:38:07 PDT
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 User image Erik Vold [:erikvold] (please needinfo? me) 2012-08-21 17:53:34 PDT
I like the idea Alex, I just want to confirm the tests pass on Fennec before pulling.
Comment 10 User image [github robot] 2012-08-29 06:53:24 PDT
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
Comment 11 User image Mark Hammond [:markh] 2012-10-29 22:45:19 PDT
*** Bug 765005 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.