Closed Bug 1245074 Opened 4 years ago Closed 4 years ago

'Restore Defaults' button via Customize mode is enabled by default with a clean profile

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 48
Tracking Status
firefox44 --- unaffected
firefox45 --- unaffected
firefox46 + verified
firefox47 + verified
firefox48 --- verified

People

(Reporter: adalucinet, Assigned: mixedpuppy)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(3 files, 2 obsolete files)

Affected versions: latest Aurora 46.0a2 and Nightly 47.0a1 (from 2016-02-01), e10s enabled/disabled
Affected platforms: Ubuntu 14.04 32-bit, Mac OS X 10.9.5 and Windows 7 64-bit

Steps to reproduce:
1. Launch Firefox with a clean profile.
2. Click on Menu panel (Hamburger button).
3. Select Customize.

Expected results: 'Restore Defaults' is disabled.
Actual results: 'Restore Defaults' button is enabled by default. 

Additional notes:
1. When 'Restore Defaults' button is clicked, Pocket button is moved to Additional Tools and Features.
2. Unaffected version - 45 beta 2 (Build ID: 20160201143558).
3. Regression range:
Last good revision: 0c12d4229be0068eb1b9beb5064b800f12143f20
First bad revision: 3ba655f6bc67660a2dcfc4c2a5b3d0d17714f53d
Pushlog: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=0c12d4229be0068eb1b9beb5064b800f12143f20&tochange=3ba655f6bc67660a2dcfc4c2a5b3d0d17714f53d

Regressed by 3ba655f6bc67 Shane Caraveo — Bug 1215694 move pocket to a system addon
Additional notes 4: Intermittently, the browser is blocked in customize mode; Please take a look at this screen recording: https://goo.gl/6qnSeO ; reproducible only if Pocket button is moved to the menu panel; this issue is also in the same regression range.
Oh yikes, thanks for the great screencast adalucinet! Very illustrative.

Hey mixedpuppy, do you know what's happening here?
Flags: needinfo?(mixedpuppy)
No longer blocks: 1215694
Depends on: 1215694
Depends on: 1235627
CUI needs an api that a widget in a system addon can use to say, hey! make me a default.
Flags: needinfo?(mixedpuppy)
(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> CUI needs an api that a widget in a system addon can use to say, hey! make
> me a default.

Is what is being proposed in bug 1023319 what you're looking for?
Flags: needinfo?(mixedpuppy)
That may do it.  I was thinking that in combination with bug 1232222 would work well.  However, I just realized there may be some problem/edge cases with the socialapi widgets since (other than share) they are not defined in CustomizableWidgets.jsm (are defined in Social.jsm)
Flags: needinfo?(mixedpuppy)
Cool - tentatively marking dependency then.

adalucinet, thanks for this. comment 1, however, describes a more serious bug that I think should be separate from this one. Would you mind filing it separately?
Depends on: 1023319
Flags: needinfo?(alexandra.lucinet)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #6)
> adalucinet, thanks for this. comment 1, however, describes a more serious
> bug that I think should be separate from this one. Would you mind filing it
> separately?

Sure thing! The issue mentioned in comment 1 is now tracked in bug 1245447; after further investigations, found another 2 edge-cases: bug 1245445 and bug 1245444 (weird that is not reproducible with 46 Aurora), regressions from the same bug.
Flags: needinfo?(alexandra.lucinet)
Keywords: regression
Assignee: nobody → mixedpuppy
Platform Triage: Can you take a look at this? This is called out as a recent regression that we're tracking.
Assignee: mixedpuppy → nobody
Flags: needinfo?(mixedpuppy)
(In reply to Al Billings [:abillings] from comment #8)
> Platform Triage: Can you take a look at this? This is called out as a recent
> regression that we're tracking.

FWIW, see discussion in bug 1235627...
Shane, can you back out or get a fix asap.
This is the simplest patch I could think of, keeping in mind this needs beta uplift.
Flags: needinfo?(mixedpuppy)
Attachment #8732650 - Flags: review?(gijskruitbosch+bugs)
[Tracking Requested - why for this release]: fixes a regression in customization mode.
Comment on attachment 8732650 [details] [diff] [review]
workaround CUI defaults for pocket to fix restore defaults

Review of attachment 8732650 [details] [diff] [review]:
-----------------------------------------------------------------

Why aren't we fixing this as discussed in bug 1023319?

::: browser/extensions/pocket/bootstrap.js
@@ +517,5 @@
> +  setDefaultPrefs();
> +  // migrate enabled pref
> +  if (Services.prefs.prefHasUserValue("browser.pocket.enabled")) {
> +    Services.prefs.setBoolPref("extensions.pocket.enabled", Services.prefs.getBoolPref("browser.pocket.enabled"));
> +    Services.prefs.clearUserPref("browser.pocket.enabled");

Why are we moving this?
Attachment #8732650 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #14)
> Comment on attachment 8732650 [details] [diff] [review]
> workaround CUI defaults for pocket to fix restore defaults
> 
> Review of attachment 8732650 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Why aren't we fixing this as discussed in bug 1023319?

Err, meant bug 1235627.
(In reply to :Gijs Kruitbosch from comment #14)
> Comment on attachment 8732650 [details] [diff] [review]
> workaround CUI defaults for pocket to fix restore defaults
> 
> Review of attachment 8732650 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Why aren't we fixing this as discussed in bug 1023319?

Because that is a more involved change affecting more code in firefox.  This needs beta uplift, lets just keep it simple for that, deal with the right fix later.

> 
> ::: browser/extensions/pocket/bootstrap.js
> @@ +517,5 @@
> > +  setDefaultPrefs();
> > +  // migrate enabled pref
> > +  if (Services.prefs.prefHasUserValue("browser.pocket.enabled")) {
> > +    Services.prefs.setBoolPref("extensions.pocket.enabled", Services.prefs.getBoolPref("browser.pocket.enabled"));
> > +    Services.prefs.clearUserPref("browser.pocket.enabled");
> 
> Why are we moving this?

Per the comment I added, the addon prefs need to be set set prior to CUI initialization.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Shane Caraveo (:mixedpuppy) from comment #16)
> (In reply to :Gijs Kruitbosch from comment #14)
> > Comment on attachment 8732650 [details] [diff] [review]
> > workaround CUI defaults for pocket to fix restore defaults
> > 
> > Review of attachment 8732650 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Why aren't we fixing this as discussed in bug 1023319?
> 
> Because that is a more involved change affecting more code in firefox.  This
> needs beta uplift,

I disagree that this needs uplift. If it did it should have been addressed sooner. Backing out the pocket-as-system-addon change seems safer to me.

Even uplifting this change scares me - it's hacky and likely to lead to problems. You're not enabling the pref for tests, either, so there's no verification of how this affects anything.

> lets just keep it simple for that, deal with the right
> fix later.

I don't think this is simple, and I think we've been delaying the 'right' fix for too long already. We should fix this properly.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #17)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #16)
> > (In reply to :Gijs Kruitbosch from comment #14)
> > > Comment on attachment 8732650 [details] [diff] [review]
> > > workaround CUI defaults for pocket to fix restore defaults
> > > 
> > > Review of attachment 8732650 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > Why aren't we fixing this as discussed in bug 1023319?
> > 
> > Because that is a more involved change affecting more code in firefox.  This
> > needs beta uplift,
> 
> I disagree that this needs uplift. If it did it should have been addressed
> sooner. Backing out the pocket-as-system-addon change seems safer to me.

Pocket was added to firefox without tests and was deeply integrated in a number of places.  Backing out is going to touch a ton of code in Firefox that has possibly shifted and changed in the 3 months since the addon landed.  It also still wont provide any test coverage since there were no pocket tests.

This change is 2 lines of change in core firefox code and leaves is with an addon that could be separately/remotely disabled and/or updated.

Given this situation, IMO, smaller is better.

> Even uplifting this change scares me - it's hacky and likely to lead to
> problems. You're not enabling the pref for tests, either, so there's no
> verification of how this affects anything.

This change does exactly the same thing as dev tools and web ide, checks a pref to insert an id into the "defaults" list, so IMO it is no more or less hacky than those.  As far as tests go, given nobody cared about them when pocket was initially integrated (or any time after), so there is no verification of how backing out will affect anything.  

However, given the patch here, for your reading pleasure (I'm looking into the ui tour test):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4f5c4d399d5&selectedJob=18332632


So somebody will need to decide which way to go since we're at an impasse.
Flags: needinfo?(abillings)
I feel I also need to ask the question, is this regression really important enough to warrant either action?  Having the reset button enabled on new profiles doesn't really break anything, if a user resets it just moves the pocket button to the customization palette.
(In reply to Shane Caraveo (:mixedpuppy) from comment #18)
> (In reply to :Gijs Kruitbosch from comment #17)
> > (In reply to Shane Caraveo (:mixedpuppy) from comment #16)
> > > (In reply to :Gijs Kruitbosch from comment #14)
> > > > Comment on attachment 8732650 [details] [diff] [review]
> > > > workaround CUI defaults for pocket to fix restore defaults
> > > > 
> > > > Review of attachment 8732650 [details] [diff] [review]:
> > > > -----------------------------------------------------------------
> > > > 
> > > > Why aren't we fixing this as discussed in bug 1023319?
> > > 
> > > Because that is a more involved change affecting more code in firefox.  This
> > > needs beta uplift,
> > 
> > I disagree that this needs uplift. If it did it should have been addressed
> > sooner. Backing out the pocket-as-system-addon change seems safer to me.
> 
> Pocket was added to firefox without tests and was deeply integrated in a
> number of places.  Backing out is going to touch a ton of code in Firefox
> that has possibly shifted and changed in the 3 months since the addon
> landed.  It also still wont provide any test coverage since there were no
> pocket tests.

I'm not talking about pocket tests, I'm talking about customization, which is explicitly what this bug is about. There are a lot of tests for that, and pocket is currently entirely disabled in the testing infrastructure. You're not re-enabling it, so there's no automation to verify that your change fixes this bug, and/or what other customization issues it might cause.

> This change is 2 lines of change in core firefox code and 

... and a bunch of changes to the initialization of the add-on that has been changed numerous times since it was written because we kept finding new edgecases.

> > Even uplifting this change scares me - it's hacky and likely to lead to
> > problems. You're not enabling the pref for tests, either, so there's no
> > verification of how this affects anything.
> 
> This change does exactly the same thing as dev tools and web ide, checks a
> pref to insert an id into the "defaults" list, so IMO it is no more or less
> hacky than those.

Maybe not, but the add-on initialization changes are also a factor here, as is the fact that the tests are still effectively disabled.

>  As far as tests go, given nobody cared about them when
> pocket was initially integrated (or any time after), 

This is not true. When I reviewed the patches on the integration bug (bug 1215694), no test changes were in the patches, and none were reviewed. You added those changes when landing, and landed with pocket disabled for the tests, without explicitly noting this in the bug. I only realized much later that that had happened. Other Firefox peers have since been surprised (and that is a mild expression) that this is how this was landed, including this and other regressions. It's on me that I didn't publicly call you out on this earlier, but I assumed this was going to be addressed sooner. Yet here we are. 

> However, given the patch here, for your reading pleasure (I'm looking into
> the ui tour test):
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=e4f5c4d399d5&selectedJob=18332632

and the leaks in the save link test? Or are those from the ancestry of that patch set?
(In reply to :Gijs Kruitbosch from comment #17)

> Even uplifting this change scares me - it's hacky and likely to lead to
> problems. You're not enabling the pref for tests, either, so there's no
> verification of how this affects anything.
> 
> I don't think this is simple, and I think we've been delaying the 'right'
> fix for too long already. We should fix this properly.

What kind of problems are you thinking of?   

I am also concerned about the tests, discussed in bug 1235627. If we want to set up a structure to fast track changes for this system addon, we should have good automated test coverage. But we still don't.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #21)
> (In reply to :Gijs Kruitbosch from comment #17)
> 
> > Even uplifting this change scares me - it's hacky and likely to lead to
> > problems.
> What kind of problems are you thinking of?   

Issues that this could potentially affect/reintroduce:
- bug 1248780
- bug 1255824
- bug 1252661

and this bug also impacts bug 1248780, in particular because AIUI this solution won't fix it, but addressing the CUI deficiency in dealing with system add-ons on a systemic level could potentially address this.
Blocks: 1235627, 1215694
No longer depends on: 1235627, 1215694
(In reply to :Gijs Kruitbosch from comment #20)

> > This change is 2 lines of change in core firefox code and 
> 
> ... and a bunch of changes to the initialization of the add-on that has been
> changed numerous times since it was written because we kept finding new
> edgecases.

The important items are having extensions.pocket.enabled set to true prior to CUI initialization, and adding pocket-button to the default list.  That fixes the regression.  The patch could be simplified to just add it to the list (not conditionally based on the pref) and add the enabled pref to firefox.js.  Changes in the addon wouldn't be necessary, but the test fixes I'm working (in the try below) on would be.

> >  As far as tests go, given nobody cared about them when
> > pocket was initially integrated (or any time after), 
> 
> This is not true. 

I rechecked to make sure I wasn't crazy.  Outside of UITour there were no pocket tests.  But as you said earlier you are talking about CUI tests, not pocket tests.  There was nothing specific to pocket in CUI tests either, only the isdefault stuff that tests tend to check after doing a CUI reset.  That is just a test that the button id's in the default list are in the ui in a certain position.

The important point to all this is: backing out the addon (thus back to the builtin code) will not give us any additional tests or assurances, and is a much larger change than adding the button into the default list.  The old code is likely out of sync with other changes in the code base and there is no way to know what that may be or what issues it may introduce.

Yes, I should have turned on the addon during tests earlier than now, maybe that would have caught an item or two, but certainly not everything (e.g. missing button icon on linux).

> > However, given the patch here, for your reading pleasure (I'm looking into
> > the ui tour test):
> > 
> > https://treeherder.mozilla.org/#/
> > jobs?repo=try&revision=e4f5c4d399d5&selectedJob=18332632
> 
> and the leaks in the save link test? Or are those from the ancestry of that
> patch set?

I'll have to look if that is an issue in earlier tests.  That leak bug has been there a long time, but appears to have gone away for quite a while as well.  I ran the try to see what tests would break so I can tackle those.
This approach requires tests turned on, see test patch
fixes in addon:
- One stupid bug was found.  
- One "leak" was found, a test complained about pocket loading pktApi.js when necessary.  
- One context menu access key conflicted with some other context menu item, found one that passes tests.
Attachment #8733152 - Attachment is patch: true
Flags: needinfo?(abillings)
(In reply to Shane Caraveo (:mixedpuppy) from comment #23)
> (In reply to :Gijs Kruitbosch from comment #20)
> 
> > > This change is 2 lines of change in core firefox code and 
> > 
> > ... and a bunch of changes to the initialization of the add-on that has been
> > changed numerous times since it was written because we kept finding new
> > edgecases.
> 
> The important items are having extensions.pocket.enabled set to true prior
> to CUI initialization, and adding pocket-button to the default list.  That
> fixes the regression.  The patch could be simplified to just add it to the
> list (not conditionally based on the pref) and add the enabled pref to
> firefox.js.  Changes in the addon wouldn't be necessary, but the test fixes
> I'm working (in the try below) on would be.

It seems simpler to just add the pref to firefox.js and make the addition of the item to the defaults conditional on the pref still. That'd still be a smaller patch and wouldn't require test fixes, right?

> > >  As far as tests go, given nobody cared about them when
> > > pocket was initially integrated (or any time after), 
> > 
> > This is not true. 
> 
> I rechecked to make sure I wasn't crazy.  Outside of UITour there were no
> pocket tests.  But as you said earlier you are talking about CUI tests, not
> pocket tests.  There was nothing specific to pocket in CUI tests either,
> only the isdefault stuff that tests tend to check after doing a CUI reset. 
> That is just a test that the button id's in the default list are in the ui
> in a certain position.

... which would have caught this bug, as well as bug 1245447 and its dupes. Mozscreenshots (which we now have on infra) might have helped with figuring out some of the icon issues. As it is, we caught none of this in infra, it was found by QA after landing and was only fixed a cycle later and uplifted (or is being fixed now, in this bug). If Pocket hadn't been disabled for tests, we'd have had to fix all this when landing it initially. Might have taken longer at the time, but would have been better for quality.

> > > However, given the patch here, for your reading pleasure (I'm looking into
> > > the ui tour test):
> > > 
> > > https://treeherder.mozilla.org/#/
> > > jobs?repo=try&revision=e4f5c4d399d5&selectedJob=18332632
> > 
> > and the leaks in the save link test? Or are those from the ancestry of that
> > patch set?
> 
> I'll have to look if that is an issue in earlier tests.  That leak bug has
> been there a long time,

? I don't see a bug on file with:

TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_save_private_link_perwindowpb.js | leaked 1 window(s) until shutdown [url = about:blank]


and the .properties change is l10n so we can't uplift that. Sounds like we can't enable the tests on 46 (or have to make specific additions to those tests to disable the feature for those tests) if we're uplifting.

(In reply to Shane Caraveo (:mixedpuppy) from comment #19)
> I feel I also need to ask the question, is this regression really important
> enough to warrant either action?  Having the reset button enabled on new
> profiles doesn't really break anything, if a user resets it just moves the
> pocket button to the customization palette.

I would personally agree with Shane and don't think this should be uplifted to 46. I think we should fix it properly and uplift that to 47.
(In reply to :Gijs Kruitbosch from comment #27)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #23)
> > (In reply to :Gijs Kruitbosch from comment #20)

> > The important items are having extensions.pocket.enabled set to true prior
> > to CUI initialization, and adding pocket-button to the default list.  That
> > fixes the regression.  The patch could be simplified to just add it to the
> > list (not conditionally based on the pref) and add the enabled pref to
> > firefox.js.  Changes in the addon wouldn't be necessary, but the test fixes
> > I'm working (in the try below) on would be.
> 
> It seems simpler to just add the pref to firefox.js and make the addition of
> the item to the defaults conditional on the pref still. That'd still be a
> smaller patch and wouldn't require test fixes, right?

Right, though I'd like to get the one line fix in the addon that I found as well.


> > I'll have to look if that is an issue in earlier tests.  That leak bug has
> > been there a long time,
> 
> ? I don't see a bug on file with:
> 
> TEST-UNEXPECTED-FAIL |
> browser/base/content/test/general/browser_save_private_link_perwindowpb.js |
> leaked 1 window(s) until shutdown [url = about:blank]

Yeah, I mistook a bug listed under that as being related.  I spent a bunch of time on this last night.    I can't see anything that the addon is doing to affect this, but it definitely leaks when the addon is enabled in tests.

> and the .properties change is l10n so we can't uplift that. Sounds like we
> can't enable the tests on 46 (or have to make specific additions to those
> tests to disable the feature for those tests) if we're uplifting.

Right.  I could make a small change in the test for that to skip the pocket menuitem, there is already a list of menus that are skipped on it.
(In reply to :Gijs Kruitbosch from comment #27)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #19)
> > I feel I also need to ask the question, is this regression really important
> > enough to warrant either action?  Having the reset button enabled on new
> > profiles doesn't really break anything, if a user resets it just moves the
> > pocket button to the customization palette.
> 
> I would personally agree with Shane and don't think this should be uplifted
> to 46. I think we should fix it properly and uplift that to 47.

It would be great to get an opinion on that asap, it would make a big difference in how I spend my time on this issue.
Flags: needinfo?(lhenry)
Pref checked in cui.  Bug fix applied in addon found during testing.  Because the test system has a pref that overrides the default, addon will not be enabled during tests with this patch alone.
Assignee: nobody → mixedpuppy
Attachment #8732650 - Attachment is obsolete: true
Attachment #8733152 - Attachment is obsolete: true
Attachment #8733496 - Flags: review?(gijskruitbosch+bugs)
Attachment #8733496 - Attachment is patch: true
Comment on attachment 8733496 [details] [diff] [review]
v2: workaround CUI defaults for pocket to fix restore defaults

Review of attachment 8733496 [details] [diff] [review]:
-----------------------------------------------------------------

This seems sane for uplift. I'd still want to get the 'real' fix prioritized for 47, though.
Attachment #8733496 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/integration/fx-team/rev/b153d20c301d0525db7248a23c13e862779bf9a5
Bug 1245074 workaround CUI defaults for pocket to fix restore defaults, r=gijs
https://hg.mozilla.org/mozilla-central/rev/b153d20c301d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment on attachment 8733496 [details] [diff] [review]
v2: workaround CUI defaults for pocket to fix restore defaults

Approval Request Comment
[Feature/regressing bug #]:pocket
[User impact if declined]:CUI restore button is enabled for new profiles when it should be disabled
[Describe test coverage new/current, TreeHerder]:manual verification, though this was run along with a patch that enabled pocket during test runs.
[Risks and why]: low for aurora, moderate for beta just because.
[String/UUID change made/needed]:none
Attachment #8733496 - Flags: approval-mozilla-beta?
Attachment #8733496 - Flags: approval-mozilla-aurora?
Comment on attachment 8733496 [details] [diff] [review]
v2: workaround CUI defaults for pocket to fix restore defaults

Workaround fix manually tested, ok to uplift for beta 5.
Flags: needinfo?(lhenry)
Attachment #8733496 - Flags: approval-mozilla-beta?
Attachment #8733496 - Flags: approval-mozilla-beta+
Attachment #8733496 - Flags: approval-mozilla-aurora?
Attachment #8733496 - Flags: approval-mozilla-aurora+
Backed out from beta in https://hg.mozilla.org/releases/mozilla-beta/rev/2af4ffa6fd9a for apparently breaking the build like https://treeherder.mozilla.org/logviewer.html#?job_id=932576&repo=mozilla-beta

For the record, I hit a merge conflict in the firefox.js file, maybe I didn't resolve it correctly.
Flags: needinfo?(mixedpuppy)
(In reply to Wes Kocher (:KWierso) from comment #38)
> Backed out from beta in
> https://hg.mozilla.org/releases/mozilla-beta/rev/2af4ffa6fd9a for apparently
> breaking the build like
> https://treeherder.mozilla.org/logviewer.html#?job_id=932576&repo=mozilla-
> beta
> 
> For the record, I hit a merge conflict* in the firefox.js file, maybe I
> didn't resolve it correctly.

* when first uplifting this to beta
still hitting problems like 

grafting 335920:4b14e550e7d8 "Bug 1245074 workaround CUI defaults for pocket to fix restore defaults, r=gijs a=lizzard CLOSED TREE"
merging browser/app/profile/firefox.js
merging browser/components/customizableui/CustomizableUI.jsm
merging browser/extensions/pocket/bootstrap.js
warning: conflicts while merging browser/app/profile/firefox.js! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
patch for beta.  previous merge/build error in comment 38 was due to an error in updating the patch for beta.
Flags: needinfo?(mixedpuppy)
Flags: qe-verify+
I’ve managed to still reproduce this bug on 47.0a2 (2016-03-31), using Ubuntu 12.04 x64, Ubuntu 14.04 x86 and x64, Mac OS X 10.11 Beta and Windows 10 x64 -- "Restore Defaults" button is still enabled; when it is clicked, "Pocket" button keeps its position, but "Open WebIDE" and "Hello" buttons switch theirs positions.

This bug is verified fixed on:
- 46.0b6-build1 (20160328182534),
- 48.0a1 (2016-03-31),
using Ubuntu 12.04 x64, Ubuntu 14.04 x86 and x64, Mac OS X 10.11 Beta and Windows 10 x64.
(In reply to Iulia Cristescu, QA [:IuliaC] from comment #43)
> I’ve managed to still reproduce this bug on 47.0a2 (2016-03-31), using
> Ubuntu 12.04 x64, Ubuntu 14.04 x86 and x64, Mac OS X 10.11 Beta and Windows
> 10 x64 -- "Restore Defaults" button is still enabled; when it is clicked,
> "Pocket" button keeps its position, but "Open WebIDE" and "Hello" buttons
> switch theirs positions.

Shane, did you have the chance to take a look at this issue on Fx47? The behavior mentioned in Comment 43 is still reproducible, I've just checked with 47.0a2 (2016-04-05).
Flags: needinfo?(mixedpuppy)
(In reply to Iulia Cristescu, QA [:IuliaC] from comment #44)
> (In reply to Iulia Cristescu, QA [:IuliaC] from comment #43)
> > I’ve managed to still reproduce this bug on 47.0a2 (2016-03-31), using
> > Ubuntu 12.04 x64, Ubuntu 14.04 x86 and x64, Mac OS X 10.11 Beta and Windows
> > 10 x64 -- "Restore Defaults" button is still enabled; when it is clicked,
> > "Pocket" button keeps its position, but "Open WebIDE" and "Hello" buttons
> > switch theirs positions.
> 
> Shane, did you have the chance to take a look at this issue on Fx47? The
> behavior mentioned in Comment 43 is still reproducible, I've just checked
> with 47.0a2 (2016-04-05).

This is Hello's fault, not Pocket's (and therefore not Shane's). It'll only happen on aurora / devedition, and will go away again when 47 hits beta.

Please file a separate bug.
Flags: needinfo?(mixedpuppy) → needinfo?(iulia.cristescu)
No longer blocks: 1262447
(In reply to :Gijs Kruitbosch from comment #45)
> Please file a separate bug.
Heloo, as you have already seen, I filed a separate bug 1262447 for this Aurora issue :) Thanks!
Flags: needinfo?(iulia.cristescu)
You need to log in before you can comment on or make changes to this bug.