678263 - panel.show(anchor) doesn't work, we should yank it from the docs

Status: RESOLVED FIXED

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

Description

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

Currently the docs for panel.show() state: 

"show(anchor)

Displays the panel.
[ anchor : handle ]

A handle to a DOM node in a page to which the panel should appear to be anchored. If not given, the panel is centered inside the most recent browser window. Note that it is not currently possible to anchor panels in this way using only the high level APIs."

The ability to anchor panels to content nodes is either unimplemented or broken ( see bug #677785 ) - if add ing / fixing the functionality isn't going to happen soon we should consider amending the docs to be something like this:

"show()

Displays the panel centered inside the most recent browser window."

Comment 1

[Will Bamberg [:wbamberg]]

I'm not sure I agree with this. panel.show is not broken, exactly, it works fine. The problem is that, by design, it's not possible to get at DOM nodes using the APIs in addon-kit. So the anchor argument is not useful to people who are using the high-level APIs (which the docs already say) - but that doesn't mean it's not useful, full stop.

Would it be helpful to be more explicit?

"show(anchor)

Displays the panel.
[ anchor : handle ]

A handle to a DOM node in a page to which the panel should appear to be anchored. If not given, the panel is centered inside the most recent browser window.

Add-ons which restrict themselves to the APIs in the [addon-kit package] can't make use of this argument, and so are not able to anchor panels to DOM nodes."

Comment 2

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

I hadn't bothered to hack out something like this and really understand what was going on:

https://builder.addons.mozilla.org/addon/1013359/revision/22/

I like the new wording better, maybe with a little more sugar:

Add-ons which restrict themselves to the APIs in the [addon-kit package] can't make use of this argument, and so are not able to anchor panels to DOM nodes without using lower-level apis."

Comment 3

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

(Pushing all open bugs to the --- milestone for the new triage system)

Comment 4

[Will Bamberg [:wbamberg]]

Attachment: Clarify how 'anchor' argument can be used

Comment 5

[Dietrich Ayala (:dietrich)]

Since this is a high-level API, we should not document this functionality, and should instead remove it altogether.

If we need this functionality, we should split it into an api-utils version that exposes the feature in a way that makes sense at the low-level, and a high-level version that will not break in the future.

That's the way we've done every other API, and is in line with the requirements for high-level APIs.

Comment 6

[Will Bamberg [:wbamberg]]

(In reply to Dietrich Ayala (:dietrich) from comment #5)
> Since this is a high-level API, we should not document this functionality,
> and should instead remove it altogether.
> 
> If we need this functionality, we should split it into an api-utils version
> that exposes the feature in a way that makes sense at the low-level, and a
> high-level version that will not break in the future.
> 
> That's the way we've done every other API, and is in line with the
> requirements for high-level APIs.

So shall I update this patch to remove any mention of the 'anchor' argument, and raise a separate bug to make the code changes you've proposed?

Comment 7

[Will Bamberg [:wbamberg]]

Attachment: Just remove any mention of the anchor parameter

Comment 8

[Will Bamberg [:wbamberg]]

Thanks Dietrich!

-> https://github.com/mozilla/addon-sdk/commit/25c52e72032eebc39169dba284966551c87e6d39