Closed Bug 886329 Opened 9 years ago Closed 9 years ago

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

Categories

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

x86_64
All

Tracking

(firefox23 wontfix, firefox24+ verified, firefox25+ verified, firefox26+ verified)

VERIFIED FIXED
mozilla26
Tracking Status
firefox23 --- wontfix
firefox24 + verified
firefox25 + verified
firefox26 + verified

People

(Reporter: patilkr24, Assigned: zer0)

References

Details

(Keywords: regression)

Attachments

(2 files)

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.
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?
Severity: normal → major
Status: UNCONFIRMED → NEW
Component: Untriaged → General
Ever confirmed: true
Flags: needinfo?(rFobic)
Product: Firefox → Add-on SDK
Target Milestone: --- → mozilla23
Version: 23 Branch → unspecified
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.
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?
Yes I confirm that that change caused regression, I'm still trying to figure out a reason it doesn't work.
Flags: needinfo?(rFobic)
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..
Severity: major → normal
Status: NEW → UNCONFIRMED
Ever confirmed: false
Priority: P1 → --
Target Milestone: mozilla23 → ---
Ooops, I must have accidentally pressed the wrong key...sorry
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P1
Target Milestone: --- → mozilla23
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.
OS: Linux → All
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/ .
Irakli, are you actively still looking at this?
Flags: needinfo?(rFobic)
Replied over IRC, yes!
Flags: needinfo?(rFobic)
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
Jeff I think you mentioned some workaround today, is it posted somewhere else ? Mind posting it here ?
Flags: needinfo?(jgriffiths)
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.
Flags: needinfo?(jgriffiths)
(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
(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*.
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/
The workaround did not work for 1Password in my testing.
Requesting tracking since this potentially affects many add-ons and users.
Duplicate of this bug: 906819
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.
Assignee: nobody → zer0
Can we add the bug that caused this regression in the Blocking field ?
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
Blocks: 861277
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.
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 ?
Flags: needinfo?(evold)
(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
Flags: needinfo?(evold)
(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.
(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.
Flags: needinfo?(rFobic)
Irakli, what do we lose by backing out the detraitification of panel?
(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/>`.
Flags: needinfo?(rFobic)
(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.
(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.
(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?
(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`.
Attachment #796589 - Flags: review?(rFobic)
Attachment #796589 - Flags: review?(rFobic) → review+
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
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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.
(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.
Adding verofy for QA to help with verification on a few add-ons, once this is on nightly.
Keywords: verifyme
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 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.
Attachment #796589 - Flags: approval-mozilla-beta?
Attachment #796589 - Flags: approval-mozilla-aurora?
tested this on nightly from 2013-08-30 and it works as expected.
Status: RESOLVED → VERIFIED
Attachment #796589 - Flags: approval-mozilla-beta?
Attachment #796589 - Flags: approval-mozilla-beta+
Attachment #796589 - Flags: approval-mozilla-aurora?
Attachment #796589 - Flags: approval-mozilla-aurora+
(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 !
How does one export an easy to apply patch from a Github commit?
(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 zer0@mozilla.com, 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.
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.
(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
Flags: needinfo?(zer0)
Flags: needinfo?(rFobic)
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.
Flags: needinfo?(zer0)
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?
I believe it just missed the fix for Firefox 24b8, but it should be there for the next 24 beta.
You need to log in before you can comment on or make changes to this bug.