886329 - drop-down list in Jetpack add-on breaks entire UI in Aurora (23.0a2)

Status: VERIFIED FIXED

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

Description

[Kailas]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.110 Safari/537.36

Steps to reproduce:

1. Install "drop-downtext.xpi" from https://github.com/patilkr/drop-downTestCode
2. In add-on bar click on shield icon to open UI,
3. Select drop-down box and it breaks UI in Aurora *23.0a2), whereas it works correctly in FF v21.



Actual results:

UI of add-on goes blank after click on drop-down box. 


Expected results:

drop-down list should have displayed without breaking the add-on UI.

Comment 1

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

According to mozregression, this broke between the April 14 and April 15 Nightly builds, in this range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ef802a6418f2&tochange=261d6997d1d1

Bug 861277's uplift is included in there, which involved these changes to the SDK's libraries: https://github.com/mozilla/addon-sdk/compare/99d7f27c7e51...566553d48ed3

As a first guess, I'd suspect the removal of traits from the panel implementation?

Does that seem possible, Irakli?

Comment 2

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

Also, just as an observation, when I moved Firefox to a secondary monitor before clicking the dropdown, the actual dropdown list appeared on my primary monitor, though the rest of the panel's contents had vanished.

Comment 3

[Frederik Braun [:freddy]]

Yes, the dropdown suddenly moves to the left and the panel content vanishes.
Is there something else we (Kailas and I) can do to help fixing this?

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Yes I confirm that that change caused regression, I'm still trying to figure out a reason it doesn't work.

Comment 5

[Frederik Braun [:freddy]]

We may want to demo an add-on at a conference next week, that breaks due to this bug.
Is there anything we can help debugging this behaviour? I am still left clueless as there are no error messages or failures in either error console or cli output..

Comment 6

[Frederik Braun [:freddy]]

Ooops, I must have accidentally pressed the wrong key...sorry

Comment 7

[Tiziana Sellitto [:tiziana]]

It doesn't work on Mac,too.
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20130718 Firefox/25.0

I don't know if it helps, but I've noticed that if I use keyboard (tab + up/down) the UI doesn't break. The problem appears only if I click on the drop-down list.

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

This still happens with the latest Nightly, on mac and windows both. I am testing it with tiziana's addon, at https://builder.addons.mozilla.org/package/198709/latest/ .

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Attachment: Dropdown menu displayed to the left while add-on panel goes blank

Comment 12

[Dave Townsend [:mossop]]

Irakli, are you actively still looking at this?

Comment 13

[Dave Townsend [:mossop]]

Replied over IRC, yes!

Comment 14

[joehopf]

Hi,

this bug also breaks my addon, and since resolution is still pending I wanted to tell you my temporary fix:

If possible to give the select element a size attribute and set it to for example 5, then it works. Hope this is a solution for some until the issue is resolved,

Best Regards,
Joseph Hopfgartner

Comment 15

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Jeff I think you mentioned some workaround today, is it posted somewhere else ? Mind posting it here ?

Comment 16

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

The workaround we talked about today is apparently if you set the size of the panel, it works fine. This needs to be tested, I have not done that.

Comment 17

[Dave Townsend [:mossop]]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #15)
> Jeff I think you mentioned some workaround today, is it posted somewhere
> else ? Mind posting it here ?

See comment 14

Comment 18

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

(In reply to Dave Townsend (:Mossop) from comment #17)
> (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from
> comment #15)
> > Jeff I think you mentioned some workaround today, is it posted somewhere
> > else ? Mind posting it here ?
> 
> See comment 14

Ah right, the size of the *select element*.

Comment 19

[dmitry.velichko]

Actually, that bug affects my extension (can't publish it for 23.0). 
It happens with SDK version 1.14 and Firefox version 23.0.1 on Windows 7, and a Linux  Firefox v. 23.0 (SDK 1.14)
code to reproduce the problem:

https://builder.addons.mozilla.org/package/200069/latest/

Comment 20

[Jamie Phelps]

The workaround did not work for 1Password in my testing.

Comment 22

[Jorge Villalobos [:jorgev] (he/him)]

Requesting tracking since this potentially affects many add-ons and users.

Comment 24

[joehopf]

it appears that another bug has been mistakenly closed as duplicate, the workaround was intenedet for that closed bug https://bugzilla.mozilla.org/show_bug.cgi?id=906819 
and also works there.

since it was closed i posted my workaround here, but now looking at it i think this is actually not a duplicate bug.

(In reply to Jamie Phelps from comment #20)
> The workaround did not work for 1Password in my testing.

Comment 25

[bhavana bajaj [:bajaj]]

Can we add the bug that caused this regression in the Blocking field ?

Comment 26

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/ef802a6418f2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130413 Firefox/23.0 ID:20130413182727
Bad:
http://hg.mozilla.org/mozilla-central/rev/7973e5a69dc5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130414 Firefox/23.0 ID:20130414144227
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ef802a6418f2&tochange=7973e5a69dc5

Regressed by:
3a62020b78ac	Wes Kocher — Bug 861277 - Uplift addon-sdk's integration branch

Comment 27

[johannes]

I have tried the "select size" work-around, and from what I could see, the idea is to set the "size" to something different than 1 ;)

<select size="1"></select> still has the same issue.
"select" with size > 1 circumvents the issue because it does not cause a drop-down in the first place.

Comment 28

[bhavana bajaj [:bajaj]]

Hey Erik - this may be a fallout from 816257.

Given comment# 22 this is impacting a lot of add-ons, can you please help with investigation here ?

Comment 30

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to bhavana bajaj [:bajaj] from comment #28)
> Hey Erik - this may be a fallout from 816257.
> 
> Given comment# 22 this is impacting a lot of add-ons, can you please help
> with investigation here ?

Yes it is.  This pull caused the regression: https://github.com/mozilla/addon-sdk/pull/928/files

Comment 31

[bhavana bajaj [:bajaj]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #30)
> (In reply to bhavana bajaj [:bajaj] from comment #28)
> > Hey Erik - this may be a fallout from 816257.
> > 
> > Given comment# 22 this is impacting a lot of add-ons, can you please help
> > with investigation here ?
> 
> Yes it is.  This pull caused the regression:
> https://github.com/mozilla/addon-sdk/pull/928/files
So do we have the next steps on how we plan to resolve this in Fx24, given we understand where the issue is ? We are already in the fourth week of beta's with very few beta's left before Fx24 releases hence the urgency here.

Comment 32

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #30)
> (In reply to bhavana bajaj [:bajaj] from comment #28)
> > Hey Erik - this may be a fallout from 816257.
> > 
> > Given comment# 22 this is impacting a lot of add-ons, can you please help
> > with investigation here ?
> 
> Yes it is.  This pull caused the regression:
> https://github.com/mozilla/addon-sdk/pull/928/files

I should clarify, I suspect the above pull caused the regression.  I forgot that the plls in bug 820953 could also be a cause for the regression.  Irakli would know more than I.

Comment 33

[Dave Townsend [:mossop]]

Irakli, what do we lose by backing out the detraitification of panel?

Comment 34

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Dave Townsend (:Mossop) from comment #33)
> Irakli, what do we lose by backing out the detraitification of panel?

I think a lot of patches has landed on top of that, and I don't think we can back this out without having to back all of them out.

To address Erik's question, I have pointed out on last weekly that I suspect that regression has to be related to the fact that we switched to an iframe we swap frame loaders with that is under `data:application/xml+xul,<window/>`.

Comment 35

[Gabor Krizsanits (INACTIVE)]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #34)
> I think a lot of patches has landed on top of that, and I don't think we can
> back this out without having to back all of them out.

Why exactly was patch that caused known regression not backed out earlier if there is no easy fix? This problem is 2 months old...

> 
> To address Erik's question, I have pointed out on last weekly that I suspect
> that regression has to be related to the fact that we switched to an iframe
> we swap frame loaders with that is under
> `data:application/xml+xul,<window/>`.

Maybe I'm just overestimating the risk here, but based on the comments it looks like we have a regressions that breaks a lot of add-ons and will be releases soon. You know which patch caused it, yet I see no progress here in the last 2 months and then the bug is assigned to Matteo for some reason. So what is the strategy here, is anyone working on this actively now at least? I don't want to be harsh, just would really like this problem to be fixed before release, I'm willing to help, but also I would like to understand how we even got to a situation like this.

Comment 36

[Dave Townsend [:mossop]]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #35)
> (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from
> comment #34)
> > I think a lot of patches has landed on top of that, and I don't think we can
> > back this out without having to back all of them out.
> 
> Why exactly was patch that caused known regression not backed out earlier if
> there is no easy fix? This problem is 2 months old...

Even two months ago this would have been difficult to back out, but it's a valid point we should have put a much higher priority on figuring out this problem. Unfortunately it's sometimes difficult to gauge how many add-ons a given issue will affect and I we underestimated this.

> > 
> > To address Erik's question, I have pointed out on last weekly that I suspect
> > that regression has to be related to the fact that we switched to an iframe
> > we swap frame loaders with that is under
> > `data:application/xml+xul,<window/>`.
> 
> Maybe I'm just overestimating the risk here, but based on the comments it
> looks like we have a regressions that breaks a lot of add-ons and will be
> releases soon. You know which patch caused it, yet I see no progress here in
> the last 2 months and then the bug is assigned to Matteo for some reason. So
> what is the strategy here, is anyone working on this actively now at least?
> I don't want to be harsh, just would really like this problem to be fixed
> before release, I'm willing to help, but also I would like to understand how
> we even got to a situation like this.

We have a fix, it should be in the tree today.

Comment 37

[Gabor Krizsanits (INACTIVE)]

(In reply to Dave Townsend (:Mossop) from comment #36)
> We have a fix, it should be in the tree today.

\o/

Just out of curiosity since I've just started working on this, what was the problem? Was it that the SDK mishandled the onpopupshowing event coming from the drop-down box? Or something else?

Comment 38

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

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #37)

> Just out of curiosity since I've just started working on this, what was the
> problem? Was it that the SDK mishandled the onpopupshowing event coming from
> the drop-down box? Or something else?

Yes, so, the drop-down fired an onpopupshowing event as well; and it was propagated to the panel: then the SDK was "thinking" that it was emitted by panel itself instead, and swapped the frames, causing the weird behavior of this bug.

Now we ignoring everything is not coming from the panel itself, in the listener for `onpopupshowing`.

Comment 39

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

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1221/files

Pointer to Github pull-request

Comment 40

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/214ad1007b9da5998ecff79752c2d53413ca0de0
Bug 886329 - drop-down list in Jetpack add-on breaks entire UI in Aurora (23.0a2)

- Ignored `popupshowing` events are not emitted by panel
- Added unit test
- Added `ready` helper function

https://github.com/mozilla/addon-sdk/commit/57d9155f932f103d60f173a3224df6b377c22df2
Merge pull request #1221 from ZER0/panel/886329

fix Bug 886329 - drop-down list in Jetpack add-on breaks entire UI in Aurora (23.0a2) r=@gozala

Comment 41

[Dave Townsend [:mossop]]

Next steps here are to uplift this code to m-c (should happen today). Get some nightlies out to verify it resolves the problem for add-ons that can reproduce. Then create patches to uplift to aurora/beta.

Comment 42

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

(In reply to Dave Townsend (:Mossop) from comment #41)
> Next steps here are to uplift this code to m-c (should happen today). Get
> some nightlies out to verify it resolves the problem for add-ons that can
> reproduce. Then create patches to uplift to aurora/beta.

Pushed the uplift to fx-team: https://hg.mozilla.org/integration/fx-team/rev/d12549fa09c0
Should make its way over to m-c sometime today, and into either tonight's or tomorrow night's nightly.

Comment 43

[bhavana bajaj [:bajaj]]

Adding verofy for QA to help with verification on a few add-ons, once this is on nightly.

Comment 44

[Tiziana Sellitto [:tiziana]]

I've tested it with link test app of comment 19 , with my app (comment 8) and the app in comment 0 and it works fine with Nightly 26.0a1 (2013-08-29)

Comment 45

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

Comment on attachment 796589 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1221/files

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Removal of traits from the implementation of Jetpack panels.
User impact if declined: <select> elements used in Jetpack panel content will break the entire panel when selected.
Testing completed (on m-c, etc.): Baked one day on m-c, confirmation that it fixed the bug.
Risk to taking this patch (and alternatives if risky): Low, all alternatives are riskier.
String or IDL/UUID changes made by this patch: None.

Comment 46

[Tracy Walker [:tracy]]

tested this on nightly from 2013-08-30 and it works as expected.

Comment 47

[bhavana bajaj [:bajaj]]

(In reply to joehopf from comment #14)
> Hi,
> 
> this bug also breaks my addon, and since resolution is still pending I
> wanted to tell you my temporary fix:
> 
> If possible to give the select element a size attribute and set it to for
> example 5, then it works. Hope this is a solution for some until the issue
> is resolved,
> 
> Best Regards,
> Joseph Hopfgartner

Can you please help try the latest nightly and confirm this fixes for you ? Thanks !

Comment 48

[(PTO June 19-July 7) Ryan VanderMeulen]

How does one export an easy to apply patch from a Github commit?

Comment 50

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

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #48)
> How does one export an easy to apply patch from a Github commit?

I'm having a horrible time fighting hotel wifi trying to get an easily applicable patch.

Best I think I can do tonight is point you at the raw diff of what landed on m-c for this patch: https://github.com/mozilla/addon-sdk/pull/1226.diff

This would apply cleanly to Aurora/Beta's addon-sdk/source directory.
Author would be [e-mail], and the commit message would be something like "Bug 886329 - Fix use of <select> elements in Jetpack panels. r=gozala".

I can just push this myself sometime this weekend once I'm back on stable internet if that's too much work, Ryan.

Comment 51

[(PTO June 19-July 7) Ryan VanderMeulen]

That's exactly what I as looking for. However, I'm running low on time to do it today, so it's probably best if you take care of it once you arrive back home.

Comment 52

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b8295c37fe7b
https://hg.mozilla.org/releases/mozilla-beta/rev/e90d6be6dbf6

Comment 53

[(PTO June 19-July 7) Ryan VanderMeulen]

Backed out from beta due to test failures.
https://hg.mozilla.org/releases/mozilla-beta/rev/79426114fe65

https://tbpl.mozilla.org/php/getParsedLog.php?id=27241859&tree=Mozilla-Beta

Comment 54

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

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #53)
> Backed out from beta due to test failures.
> https://hg.mozilla.org/releases/mozilla-beta/rev/79426114fe65
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=27241859&tree=Mozilla-Beta

Comment 55

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

Ryan, be sure that Mozilla Beta also have this change too:

https://github.com/mozilla/addon-sdk/commit/b98541662a73c61e095836390a60e803b17b129d#lib/sdk/panel.js

Otherwise it will fails with exactly the error shown in the log.

Comment 56

[(PTO June 19-July 7) Ryan VanderMeulen]

Thanks. Trying again :)
https://hg.mozilla.org/releases/mozilla-beta/rev/f80b4ca6b145

Comment 57

[John Nagle]

This is also breaking my "Ad Limiter" add-on; the prefs panel draws correctly until a drop-down menu is used. The drop down menu appears way outside the panel, and the panel goes blank and stays blank, just
as described above. 

In reference to comments 52-56, is this now "fixed", or did the attempted fix fail?

Comment 58

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

I believe it just missed the fix for Firefox 24b8, but it should be there for the next 24 beta.