Last Comment Bug 638142 - Add ability to anchor a panel to a widget, button, or any browser DOM element
: Add ability to anchor a panel to a widget, button, or any browser DOM element
Status: RESOLVED WONTFIX
[papercuts][triage-followup]
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: P1 enhancement with 3 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 616160 (view as bug list)
Depends on: 732294
Blocks: sdk/panel sdk/widget 853336
  Show dependency treegraph
 
Reported: 2011-03-02 10:14 PST by Mihai Sucan [:msucan]
Modified: 2013-11-08 13:32 PST (History)
21 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Mihai Sucan [:msucan] 2011-03-02 10:14:37 PST
We currently can associate widgets to panels, such that when the widget is clicked, the associated panel is opened and anchored at the widget element. However, if we define our own onClick handler for the widget, we loose the ability to open the panel anchored to the widget element.

Even if panel.show(anchor) allows us to give it an anchor element, the widget object doesn't give us any reference to the widget element.

Proposed solutions:

- the widget object should give us a reference to its addon bar element.
- if that's not acceptable, then give us a widget.panelShow() method that calls panel.show(widgetElement) for us.
Comment 1 Dietrich Ayala (:dietrich) 2011-03-02 21:34:46 PST
I think getter/setter for the panel sounds fine, and very flexible.

You could then do things like easily hide and show the panel from within event handlers, and could swap out panels.
Comment 2 Dietrich Ayala (:dietrich) 2011-03-03 05:52:13 PST
Ignore comment #1, we already have that :)

Option #1 to expose the DOM element is a non-starter for e10s.

Option #2 is feasible, but I'd rather not expand the core widget API for this if possible.

There's an option #3, which is to have panel.show() use an anchor provided to the ctor (or previously passed to show()).

This means that for anchored panels you don't have to keep passing in the anchor every time you show it, which is kind of nice. And also means we can do this in widgets:

widget.panel.show()/hide()

which is intuitive.

The only disconcerting part is that sometimes show() w/o an anchor uses and anchor, and sometimes it doesn't.

Myk, what are you thoughts on resolving this?
Comment 3 Myk Melez [:myk] [@mykmelez] 2011-03-03 15:07:58 PST
(In reply to comment #2)
> There's an option #3, which is to have panel.show() use an anchor provided to
> the ctor (or previously passed to show()).
> 
> This means that for anchored panels you don't have to keep passing in the
> anchor every time you show it, which is kind of nice. And also means we can do
> this in widgets:
> 
> widget.panel.show()/hide()
> 
> which is intuitive.
> 
> The only disconcerting part is that sometimes show() w/o an anchor uses and
> anchor, and sometimes it doesn't.

The other problem is that widgets have multiple anchors, one per browser window, and the anchor to which the panel should be anchored may not be the same one to which it was anchored last time (or the first anchor created when the widget was first instantiated).

Option #4 is to make panel.show() accept a widget object to which it should be anchored:

  widget.panel.show(widget)

This still runs into the multiple anchor problem, so the panel would have to query the widget for the anchor for, say, the widget in the topmost window (which gets into the issue of how to share private information between the widget and the panel without exposing it to API consumers).

But at least it would avoid the problem of show() without an anchor sometimes using an anchor.

Hmm, I need to ponder the API design issues here some more...

Also cc:ing Irakli for his thoughts regarding how we can implement the sharing of private information between these two APIs.
Comment 4 Dietrich Ayala (:dietrich) 2011-03-03 21:59:54 PST
> The other problem is that widgets have multiple anchors, one per browser
> window, and the anchor to which the panel should be anchored may not be the
> same one to which it was anchored last time (or the first anchor created when
> the widget was first instantiated).

Right now we mirror widgets across windows for the caller, so this fact of multiple anchors already existing is never exposed to add-on devs, it's handled inside the widget implementation.

This mirroring feature could be removed. However, for the common case, I think that's highly undesirable, as it creates a whole bunch of work for the add-on developer.

While I'd prefer not to make widgets' API even bigger, widget.show/hidePanel() seems to be the solution that addresses all issues in a way that's still syntactically clearest to the caller.
Comment 5 Mihai Sucan [:msucan] 2011-03-04 05:16:45 PST
The possibility of sharing private information between APIs/modules is quite important. We'd need a common solution across jetpack modules. Is there any bug or discussion going on about this? To find a solution.
Comment 6 Rob Campbell [:rc] (:robcee) 2011-03-07 08:53:25 PST
I don't want to conflate these two issues. Anchoring a panel to a widget is one (simple) thing that could be nicely addressed by Dietrich's solution in comment #4. widget.show/hidePanel()

For cross module API, I think we're going to want and need something in the devtools sdk, possibly residing in chrome that acts as a broker -- god help me if we reinvent COM again, but I'm hoping we don't need something that heavy.

I'll file that bug as soon as a hit the save changes button.
Comment 7 Myk Melez [:myk] [@mykmelez] 2011-03-07 09:00:53 PST
(In reply to comment #6)
> I don't want to conflate these two issues. Anchoring a panel to a widget is one
> (simple) thing that could be nicely addressed by Dietrich's solution in comment
> #4. widget.show/hidePanel()

Yup, I agree.  Let it be widget.show/hidePanel().
Comment 8 Rob Campbell [:rc] (:robcee) 2011-03-07 09:06:48 PST
created bug 639518 to talk about cross-addon communications.
Comment 9 Alexandre Poirot [:ochameau] 2011-03-22 06:22:47 PDT
*** Bug 616160 has been marked as a duplicate of this bug. ***
Comment 10 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2011-05-27 04:20:44 PDT
(In reply to comment #5)
> The possibility of sharing private information between APIs/modules is quite
> important. We'd need a common solution across jetpack modules. Is there any
> bug or discussion going on about this? To find a solution.

Mihai I have been exploring different ways to approach that and for the moment solution I find most flexible is the one I blogged about: http://jeditoolkit.com/2011/04/11/shareable-private-properties.html#post

Also here is code demonstrating how to use it:
https://github.com/Gozala/jetpack-io/tree/master/lib

I believe that could be good way to go forward about this
Comment 11 Myk Melez [:myk] [@mykmelez] 2011-06-15 12:55:29 PDT
(automatic reprioritization of 1.0 bugs)
Comment 12 Wes Kocher (:KWierso) 2011-08-10 10:39:25 PDT
Re-prioritizing all 1.1-targeted feature requests to 1.2.
Comment 13 Wes Kocher (:KWierso) 2011-09-08 12:12:27 PDT
(Pushing all open bugs to the --- milestone for the new triage system)
Comment 14 Myk Melez [:myk] [@mykmelez] 2011-11-02 15:16:02 PDT
Unassigning myself from bugs I am not actively working on.
Comment 15 Jeff Griffiths (:canuckistani) (:⚡︎) 2012-10-09 10:40:51 PDT
This is still a common problem: http://stackoverflow.com/questions/12793338/how-to-anchor-a-panel-to-a-dom-element
Comment 16 Matteo Ferretti [:zer0] [:matteo] 2013-05-29 23:40:18 PDT
Jordan, are you actively working on that?
I was wondering if it makes sense to add this functionality, now that we want to deprecate Widget. Because the current code of Widget "encapsulate" the XUL node created per view, it probably requires some changes to the Widget API too in order to implements this feature (unless we do some "hack" using the id).

If the feature is not implemented yet, I would suggest to don't add enhancement on Widget API side – we want to deprecate it - and focus on new UI components instead, and add this capability for the upcoming `Button`.
Comment 17 Jordan Santell [:jsantell] [@jsantell] (Away from Bugzilla for awhile) 2013-06-05 10:57:50 PDT
No, I'm not currently working on this -- but agreed, lets revisit this regarding all the new UI components
Comment 18 Erik Vold [:erikvold] (please needinfo? me) 2013-06-26 23:55:57 PDT
Can we close this?
Comment 19 Tim Guan-tin Chien [:timdream] (please needinfo; OOO and on leave until July) 2013-07-22 10:34:22 PDT
If we don't implement this, we will stop other legit uses of passing anchor element to the panel.

See my IME add-on [1]. You can get a sense of how the panel anchors to the element with image on [2].

[1] https://github.com/timdream/jszhuyin-firefox
[2] http://blog.timc.idv.tw/posts/firefox-addon-jszhuyin-ime/

I *could* re-implement that maybe top/left/height/width... but I will lost the arrow and will have to dup a lot of positioning code to my add-on.
Comment 20 Matteo Ferretti [:zer0] [:matteo] 2013-08-20 00:21:30 PDT
Erik, it seems to me that this bug is overlaps with bug 859216; I think we should close one of them.

About how to handle this feature, see also my comment there: https://bugzilla.mozilla.org/show_bug.cgi?id=859216#c3
Comment 21 Matteo Ferretti [:zer0] [:matteo] 2013-08-20 00:24:31 PDT
Or maybe we could keep one for high level API implementation (SDK objects, content document); and one for low level API implementation (display the panel anchored to any DOM node instance).

In that case, I would suggest to keep this one for high level, and the other one for low level. What do you think?
Comment 22 Dave Townsend [:mossop] 2013-11-08 11:59:30 PST
When Australis lands widget will be deprecated so we won't be working on this.
Comment 23 Mardeg 2013-11-08 13:32:28 PST
Info for the triage-followup:
The new button API implementation for Australis is bug 907374 which blocks
bug 907450 "Anchored Panels" that looks like the successor to this bug.

On IRC Mossop pointed to bug 787390 as a related bug of interest to follow and
bug 695913 is the tracking bug blocked both by that one and Australis.

https://forums.mozilla.org/addons/viewforum.php?f=27 seems ideal for further discussion.

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