Remove add-on bar and statusbar shims

RESOLVED FIXED in Firefox 57

Status

()

Firefox
Toolbars and Customization
RESOLVED FIXED
4 years ago
2 months ago

People

(Reporter: quicksaver, Assigned: dao)

Tracking

(Blocks: 3 bugs)

Trunk
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(URL)

MozReview Requests

()

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

Attachments

(1 attachment)

Filing this bug for organizational purposes. The add-on bar and the statusbar have already been removed from the UI. And if the current line of work is to be followed, their shims, currently in place to allow a better transition when Australis hits release, will be removed as well sometime after that.

I'm personally interested in exactly when this happens, and I'm sure other add-on developers will soon be as well. So I propose using this bug for this specific (future) work. (sorry if this is a duplicate, couldn't find it)
Blocks: 598929

Comment 1

4 years ago
The current kludge inside the main "navigator-toolbox" toolbar:
https://hg.mozilla.org/mozilla-central/file/bafaf8d078d8/browser/base/content/browser.xul#l899
   899     <!-- This is a shim which will go away ASAP. See bug 749804 for details -->
   900     <toolbar id="addon-bar" toolbar-delegate="nav-bar" mode="icons" iconsize="small">
   901       <hbox id="addonbar-closebutton"/>
   902       <statusbar id="status-bar"/>
   903     </toolbar>
Keywords: addon-compat
Whiteboard: [Australis:P-]
Blocks: 976420
No longer blocks: 942157
(Assignee)

Updated

3 months ago
Blocks: 1387013
No longer blocks: 976420, 598929
Summary: Remove add-on bar and statusbar shims as soon as reasonably possible → Remove add-on bar and statusbar shims
Version: 29 Branch → Trunk
(Assignee)

Updated

3 months ago
Mentor: dao+bmo@mozilla.com
Keywords: addon-compat → good-first-bug
Whiteboard: [Australis:P-] → [good first bug][lang=js]
(Assignee)

Updated

2 months ago
Assignee: nobody → dao+bmo
Mentor: dao+bmo@mozilla.com
Status: NEW → ASSIGNED
Keywords: good-first-bug
Whiteboard: [good first bug][lang=js]
Comment hidden (mozreview-request)
(Assignee)

Comment 3

2 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2cbf1e8acc514d5dce85ad7bc0d5155a8a4b9476
Comment on attachment 8895696 [details]
Bug 956731 - Remove legacy add-on bar and statusbar shims.

https://reviewboard.mozilla.org/r/166982/#review172170

Yaaay.

Can you file a followup to remove https://hg.mozilla.org/mozilla-central/file/tip/services/sync/tps/extensions/mozmill/resource/stdlib/utils.js#l278 ? I don't think any toolkit apps have an add-on bar...

::: browser/components/customizableui/CustomizableUI.jsm
(Diff revision 1)
>          "personal-bookmarks",
>        ],
>        defaultCollapsed: true,
>      }, true);
>  
> -    this.registerArea(CustomizableUI.AREA_ADDONBAR, {

Please also add a migration step into `_introduceNewBuiltinWidgets` (feel free to rename that method, its use has evolved so the name is kinda terrible now) to remove any saved placements for the area.
Attachment #8895696 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 5

2 months ago
(In reply to :Gijs from comment #4)
> Please also add a migration step into `_introduceNewBuiltinWidgets` (feel
> free to rename that method, its use has evolved so the name is kinda
> terrible now) to remove any saved placements for the area.

Given the addonbar-delegating thingy, why would there be any saved placements?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Dão Gottwald [::dao] from comment #5)
> (In reply to :Gijs from comment #4)
> > Please also add a migration step into `_introduceNewBuiltinWidgets` (feel
> > free to rename that method, its use has evolved so the name is kinda
> > terrible now) to remove any saved placements for the area.
> 
> Given the addonbar-delegating thingy, why would there be any saved
> placements?

I don't know exactly and haven't had time to dive into it, but I certainly see the placements being saved by just starting nightly on a clean profile. I expect something else is broken (like only saving customized areas, or something, or perhaps the migration from persisted currentsets?), but either way we'll need to clean up given that we would otherwise keep the information forever.
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Updated

2 months ago
Blocks: 1389034
(Assignee)

Updated

2 months ago
Blocks: 1389036
(Assignee)

Comment 7

2 months ago
(In reply to :Gijs from comment #4)
> Can you file a followup to remove
> https://hg.mozilla.org/mozilla-central/file/tip/services/sync/tps/extensions/
> mozmill/resource/stdlib/utils.js#l278 ? I don't think any toolkit apps have
> an add-on bar...

filed bug 1389036

(In reply to :Gijs from comment #6)
> I don't know exactly and haven't had time to dive into it, but I certainly
> see the placements being saved by just starting nightly on a clean profile.
> I expect something else is broken (like only saving customized areas, or
> something, or perhaps the migration from persisted currentsets?), but either
> way we'll need to clean up given that we would otherwise keep the
> information forever.

filed bug 1389034
(Assignee)

Updated

2 months ago
Attachment #8895696 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8895696 [details]
Bug 956731 - Remove legacy add-on bar and statusbar shims.

https://reviewboard.mozilla.org/r/166982/#review172210

I mean, r=me, but why not just add the migration step here? It'd be 5 lines at most...
Attachment #8895696 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 9

2 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efcedec7e0a0
Remove legacy add-on bar and statusbar shims. r=Gijs
(Assignee)

Comment 10

2 months ago
(In reply to :Gijs from comment #8)
> Comment on attachment 8895696 [details]
> Bug 956731 - Remove legacy add-on bar and statusbar shims.
> 
> https://reviewboard.mozilla.org/r/166982/#review172210
> 
> I mean, r=me, but why not just add the migration step here? It'd be 5 lines
> at most...

I'm not as familiar with these APIs as you are and I just have too much on my plate right now.

Comment 11

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/efcedec7e0a0
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in before you can comment on or make changes to this bug.