Closed Bug 1232207 Opened 4 years ago Closed 3 years ago

Permaorange browser_social_marks.js | Should be in the default state when we finish when Gecko 45 merges to aurora on 2015-12-14

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set

Tracking

(firefox45 affected)

RESOLVED WONTFIX
Tracking Status
firefox45 --- affected

People

(Reporter: philor, Assigned: mikedeboer)

References

Details

https://treeherder.mozilla.org/#/jobs?repo=try&fromchange=e283c520abef&tochange=d3b1018fcdd4&group_state=expanded&selectedJob=14559685

Hard to believe anybody changed social in the last six weeks, so it's probably some change to CustomizableUI.
Didn't notice until a try push switching the is() to todo() forced me to, but it's actually not failing on OS X, or, it seems (deeply oddly), Linux64 debug. Linux32, Linux64 opt, and ASan all fail, but not Linux64 debug. Shrug.
This is probably the sync button changes.
Flags: needinfo?(gijskruitbosch+bugs)
Blocks: 1233066
I can't reproduce locally on Windows or OS X. Going to build on Linux but that's a VM so it'll take a while to finish. In the meantime, pushed to try with logging enabled so hopefully that will tell us more about what is actually "not default" in this case.

remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=d340167a77ad
Heh, I just realized I can run my local tests against a build produced by the trypush from comment #0, and that reproduces... Poking at that now.
This is because Loop is now an add-on and doesn't insert itself in the right place, but just at the end of the toolbar, irrespective of what other buttons might be at the end by the time it does this. On aurora that is the webide button, and so now their order is the wrong way around:

https://dxr.mozilla.org/mozilla-central/source/browser/components/customizableui/CustomizableUI.jsm#213-236

... which results in this test's assertion that they are correctly ordered fails:

Mike, can you take fixing this properly and reverting the test change? Bonus points if you write a test that specifically checks that Loop does the right thing.
Blocks: 1223573
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(mdeboer)
Here, let me disable that on the trunk too so you can fix-and-revert with one patch on trunk and aurora.

FWIW, reproing locally with a trunk build shouldn't require all of https://hg.mozilla.org/try/rev/b2653b46882c, I'd expect that just the hunks that change a1 to a2 in browser/config/version.txt and config/milestone.txt would be enough.
Keywords: leave-open
FYI I have a patch removing those inDefaultState checks in a couple tests that I'll land as part of bug 1215694.  Using that as a test wont work with our new run on system addons for features.
(In reply to Shane Caraveo (:mixedpuppy) from comment #9)
> FYI I have a patch removing those inDefaultState checks in a couple tests
> that I'll land as part of bug 1215694.  Using that as a test wont work with
> our new run on system addons for features.

We should investigate this, because if the system add-ons ship as part of Firefox, it's pretty unexpected for users that "restore defaults" removes them from the toolbar... In any case, for this specific issue, the diagnosis is in comment 6, and the fix is not hard on the Hello side of things. Just needs work.
I've added bug 1235627 to cover test issues around system addons.  I had started down the rabbit hole trying to fix tests affected by system addons, for now I'm disabling the pocket addon during test runs.
Mike's going to take a look at this.
Assignee: nobody → mdeboer
(In reply to :Gijs Kruitbosch from comment #6)
> Mike, can you take fixing this properly and reverting the test change? Bonus
> points if you write a test that specifically checks that Loop does the right
> thing.

/me will try to earn those bonus points!
Flags: needinfo?(mdeboer)
deprecation in fx51
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.