873527 - Panels can't be anchored to widgets in Firefox 22

Status: RESOLVED FIXED

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

Description

[u463949]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release)
Build ID: 20130516062729

Steps to reproduce:

Run the following add-on on Firefox 22 (using either SDK 1.14 or the trunk as of today):

----------

var p = require("sdk/panel").Panel({
  width:215,
  height:160,
  contentURL: "about:mozilla"
});

var w = require("sdk/widget").Widget({
  id: "mozilla-icon",
  label: "My Mozilla Widget",
  contentURL: "http://www.mozilla.org/favicon.ico",
  panel: p
});

------------------


Actual results:

When clicking the widget, the panel appears in the middle of the browser.


Expected results:

When clicking the widget, the panel appears right next to the widget (like it does with Firefox 21 or the current nightlies).

Comment 1

[Dave Townsend [:mossop]]

Matteo, can you look at this asap?

Comment 2

[Matteo Ferretti [:zer0] [:matteo]]

So it seems that Irakli implemented a workaround in panel that was based on his "Method" library; but in order to avoid to uplift all the library he emulated its behavior. Unfortunately the emulation of `implement` lacks of returning the proper value, so when the `panel.show` is called internally from `Widget`, `undefined` is used instead of the widget's DOM node as anchor.

In latest version of SDK we do not have this issue because we have the real "Method" library, and the emulation was removed.

I'm not sure where I have to submit this pull request, definitely not in Master, and because we do not have a clear / updated "tag" in our repo for Firefox 22 I decided to create a plain patch.

Wes, let me know how we can deal with that in case.

Comment 3

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: Fixes `implement` method in `sdk/view/core`

Comment 4

[Dave Townsend [:mossop]]

Which versions are affected by this? Just Firefox 22?

Can we get a testcase that verifies this?

Comment 5

[Matteo Ferretti [:zer0] [:matteo]]

Firefox 21 doesn't have the workaround mentioned; and since Firefox 23 we have the proper implementation of `Method` library, so yes, it affect only Firefox 22.

We can have a testcase that verifies this in the current master, but not something that can be uplifted to 22, due the new panel implementation without traits.

Comment 6

[Dave Townsend [:mossop]]

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #5)
> We can have a testcase that verifies this in the current master, but not
> something that can be uplifted to 22, due the new panel implementation
> without traits.

We can't get the panel's XUL element and verify that it at least appears in the same vicinity of the widget in a test? I'm not sure what this has to do with traits.

Comment 7

[Matteo Ferretti [:zer0] [:matteo]]

Before the de-traitification the XUL node is encapsulated; therefore is not possible from an external code – e.g. test – get the XUL node from a SDK panel instance. It doesn't have an id too. We could maybe do some hacky stuff to get it, but I'm not sure it's worthy just for Firefox 22.

Since Firefox 23 we have a totally different panel implementation, without traits, that uses the "Method" library mentioned above, and gives to us the XUL node for a given panel's instance, therefore we can get the XUL node and check, at least, if it's anchored somewhere – due the current Widget implementation, I'm afraid we probably can't test if it's anchored to a given Widget, for the same reason of the panel before the de-traitification process.

Comment 8

[Matteo Ferretti [:zer0] [:matteo]]

However, in case of the Widget, we have at least the id, so we can probably do something in a bit less hacky way.

Comment 9

[Alex Keybl [:akeybl]]

Can we get a beta approval nom this week and a landing before Tuesday?

Comment 10

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 752698 [details] [diff] [review]
Fixes `implement` method in `sdk/view/core`

[Approval Request Comment]
Bug caused by feature bug #: 839746
User impact if declined: add-on devs won't be able to anchor a Panel to a Widget, as expected. See <https://groups.google.com/d/msg/mozilla-labs-jetpack/cN4VGb-XlmQ/jMfUuKja8JoJ>
Testing completed (on m-c, etc.): Manually tested, we can't write an automated test for this in beta, and we can't land it on mozilla-central or aurora because they aren't affected. 
Risk to taking this patch (and alternatives if risky): None.
String or IDL/UUID changes made by this patch: None.

Comment 11

[Dave Townsend [:mossop]]

Landed: https://hg.mozilla.org/releases/mozilla-beta/rev/ba62954ef71b

Comment 12

[Paul Silaghi, QA [:pauly]]

Reproduced the problem on FF 22b2 using the testcase in comment 0.
Verified fixed FF 22b3 Win 7 x64.

Comment 14

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/ecc41929d254e483ca8bfcc7cc56005e2bb7bd07
Bug 873527 - Panels can't be anchored to widgets in Firefox 22. r=gozala, a=akeybl