Remove tab groups migration code

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: standard8, Assigned: Gijs)

Tracking

unspecified
Firefox 52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(3 attachments)

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.
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.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
(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. :-)
Small update because Yoric tells me on IRC individual OS.File operations already block shutdown, so no need to manually add a shutdown blocker.
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. :)
(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 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+
Duplicate of this bug: 1255361
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 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.
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
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
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.