The default bug view has changed. See this FAQ.

Remove tab groups migration code

RESOLVED FIXED in Firefox 52

Status

()

Firefox
General
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: standard8, Assigned: Gijs)

Tracking

unspecified
Firefox 52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

5 months ago
I discovered via eslint that one of the tests (test_TabGroupsMigrator.js) isn't fully testing everything it is meant to.

However, in talking with Gijs, this code can be removed now as it was temporary code to provide users a way forward if they had previously been using tab groups.
(Reporter)

Updated

5 months ago
Blocks: 1312407
(Assignee)

Updated

5 months ago
Summary: Remove tab migration code → Remove tab groups migration code
Is this for the mentioned test only? If not, it could also take care of bug 1255361.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

5 months ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
(Assignee)

Comment 5

5 months ago
(In reply to Luís Miguel [:quicksaver] from comment #1)
> Is this for the mentioned test only? If not, it could also take care of bug
> 1255361.

No, I'm removing everything. It's been a full ESR cycle. People should have hit this code by now if they were going to. :-)
(Assignee)

Comment 6

5 months ago
Small update because Yoric tells me on IRC individual OS.File operations already block shutdown, so no need to manually add a shutdown blocker.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Should the icon be removed in this patch from the svg only? I would think it should be removed from the pngs as well, or leave this one for consistency?

Note I don't use those icons so I don't really care either way, just a consistency-related thought that occurred to me. :)
(Assignee)

Comment 10

5 months ago
(In reply to Luís Miguel [:quicksaver] from comment #9)
> Should the icon be removed in this patch from the svg only? I would think it
> should be removed from the pngs as well, or leave this one for consistency?
> 
> Note I don't use those icons so I don't really care either way, just a
> consistency-related thought that occurred to me. :)

Removing from the png isn't really useful (unlikely it'll make any real size difference, and we'd need to update the coordinates of all the subsequent icons in the sprite which is really painful, and I'd need to image-edit all 15-odd Toolbar.png files which is then kind of a waste of time).

Comment 11

5 months ago
mozreview-review
Comment on attachment 8806367 [details]
Bug 1312406 - part 1: remove tab groups migration code,

https://reviewboard.mozilla.org/r/89824/#review89432
Attachment #8806367 - Flags: review?(dolske) → review+

Updated

5 months ago
Duplicate of this bug: 1255361

Comment 13

5 months ago
mozreview-review
Comment on attachment 8806368 [details]
Bug 1312406 and bug 1255361 - part 2: remove tab groups backup,

https://reviewboard.mozilla.org/r/89826/#review89434
Attachment #8806368 - Flags: review?(dolske) → review+

Comment 14

5 months ago
mozreview-review
Comment on attachment 8806369 [details]
Bug 1312406 - part 3: bonus - remove dead icon,

https://reviewboard.mozilla.org/r/89828/#review89436
Attachment #8806369 - Flags: review?(dolske) → review+
*sigh* I reviewed those last two parts yesterday, but guess I didn't click the "Publish" button in ReviewBoard's confusing UI. :/ Sorry.

Comment 16

5 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/465512fbb69b
part 1: remove tab groups migration code, r=Dolske
https://hg.mozilla.org/integration/autoland/rev/789a88b977b6
and bug 1255361 - part 2: remove tab groups backup, r=Dolske
https://hg.mozilla.org/integration/autoland/rev/66ab1149f151
part 3: bonus - remove dead icon, r=Dolske

Comment 17

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/465512fbb69b
https://hg.mozilla.org/mozilla-central/rev/789a88b977b6
https://hg.mozilla.org/mozilla-central/rev/66ab1149f151
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.