Last Comment Bug 614015 - Add <browser>.messageManager property
: Add <browser>.messageManager property
Status: RESOLVED FIXED
[e10s]
: dev-doc-complete
Product: Toolkit
Classification: Components
Component: XUL Widgets (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla2.0b8
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-22 11:01 PST by Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg]
Modified: 2010-12-20 12:59 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.00 KB, patch)
2010-11-22 13:40 PST, Mark Finkle (:mfinkle) (use needinfo?)
gavin.sharp: review+
Details | Diff | Splinter Review
With tests (4.10 KB, patch)
2010-12-07 07:46 PST, Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg]
benjamin: approval2.0+
Details | Diff | Splinter Review

Description Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-11-22 11:01:15 PST
I was editing message-manager docs and found that <browser>.messageManager is only implemented in Fennec, not in the core bindings. Since we're encouraging extension authors to start using the message manager now in Firefox, we should make that property available.
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2010-11-22 13:40:19 PST
Created attachment 492426 [details] [diff] [review]
patch

As Benjamin states, this property is active in Firefox 4, so it will work fine. Once this lands, I can remove the same property from the Fennec extended binding.
Comment 2 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-11-22 13:42:19 PST
I was going to write a test, too. I know things are all loosey-goosey in mobile-land, but I'd really like to make sure this actually works.
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2010-11-22 13:49:59 PST
(In reply to comment #2)
> I was going to write a test, too. I know things are all loosey-goosey in
> mobile-land, but I'd really like to make sure this actually works.

Yeah, none of our current tests would pass if this failed. I guess browser-chrome would work. A small test would be easy for me to add.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-11-30 12:35:55 PST
Comment on attachment 492426 [details] [diff] [review]
patch

r=me with a test added. I'm not exactly clear on what this can be used for by extensions in Firefox 4, and I suppose a test would help with that (and also be useful for documentation purposes).
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-11-30 12:37:45 PST
We might want to mirror it on tabbrowser as well (as we do with other browser properties like currentURI).
Comment 6 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-11-30 12:39:30 PST
I don't *think* we want to expose it on the tabbrowser. If you want to attach a behavior to all loaded tabs you want window.messageManager, not <browser>.messageManager.
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-12-05 06:59:02 PST
tabbrowser mirrors many of the properties of its selected <browser> for convenience (tabbrowser.currentURI, .docShell, .webProgress, .contentDocument, etc.) It's probably more confusing than it is convenient, though, so I don't mind not extending that pattern for this case.
Comment 8 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-12-07 07:46:07 PST
Created attachment 495829 [details] [diff] [review]
With tests
Comment 9 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-12-07 08:55:41 PST
http://hg.mozilla.org/mozilla-central/rev/7de7681aa6d3

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