bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Remove old migration step removing bookmarks-menu-button's persisted class attribute

RESOLVED FIXED in Firefox 43

Status

()

Firefox
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8649261 [details] [diff] [review]
bookmarks-menu-button.diff

We're doing this again in step 31.

The step 16 change is just a drive-by fix.
Attachment #8649261 - Flags: review?(gijskruitbosch+bugs)

Comment 1

3 years ago
Comment on attachment 8649261 [details] [diff] [review]
bookmarks-menu-button.diff

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

::: browser/components/nsBrowserGlue.js
@@ -1983,5 @@
>        // Remove persisted collapsed state from TabsToolbar.
>        xulStore.removeValue(BROWSER_DOCURL, "TabsToolbar", "collapsed");
>      }
>  
> -    if (currentUIVersion < 21) {

Please leave a comment noting 21 was made obsolete by bug <whatever>, see migration <whichever> - doing archaeology on removed code is annoying, and the missing numbers will be confusing otherwise. :-)
Attachment #8649261 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 2

3 years ago
(In reply to :Gijs Kruitbosch from comment #1)
> Please leave a comment noting 21 was made obsolete by bug <whatever>, see
> migration <whichever> - doing archaeology on removed code is annoying, and
> the missing numbers will be confusing otherwise. :-)

It really shouldn't be confusing. There are lots of other holes and that's expected since old migration steps make sense only so much time. I don't think we should add comments for removed steps.

Comment 3

3 years ago
(In reply to Dão Gottwald [:dao] from comment #2)
> (In reply to :Gijs Kruitbosch from comment #1)
> > Please leave a comment noting 21 was made obsolete by bug <whatever>, see
> > migration <whichever> - doing archaeology on removed code is annoying, and
> > the missing numbers will be confusing otherwise. :-)
> 
> It really shouldn't be confusing. There are lots of other holes and that's
> expected since old migration steps make sense only so much time. I don't
> think we should add comments for removed steps.

Alright, then ship as-is, I guess.

(I still would love to kill this whole thing for new profiles once upon a time... but not today, I guess)
https://hg.mozilla.org/mozilla-central/rev/13bb29917965
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
You need to log in before you can comment on or make changes to this bug.