With the splitting out of the navigation and refresh buttons from the location bar container we can now move the refresh button to wherever we may want. However the back and forward buttons are immovable, although you can place something between them. The reason given for locking these buttons was that a user might move them out of the toolbar and then not be able to navigate. This makes some sense but without the ability to move them within the toolbar it does almost defeat the purpose of pulling them out of the location bar container in the first place.

Can you point out if/where/how we're still showing "blocked" icons when trying to drag these items to another toolbar?

Comment on attachment 8899553 [details] Bug 1387313 - Allow dragging non-removable items (url bar, back/forward buttons) within their toolbar. https://reviewboard.mozilla.org/r/170854/#review176226 ::: browser/components/customizableui/CustomizeMode.jsm:1640 (Diff revision 1) > // Do nothing if the widget is not allowed to be removed. > if (targetArea.id == kPaletteId && > !CustomizableUI.isWidgetRemovable(draggedItemId)) { > return; This prevents the removable=false widget from moving to the palette. ::: browser/components/customizableui/CustomizeMode.jsm:1646 (Diff revision 1) > // Do nothing if the widget is not allowed to move to the target area. > if (targetArea.id != kPaletteId && > !CustomizableUI.canWidgetMoveToArea(draggedItemId, targetArea.id)) { > return; This prevents the removable=false widget from moving to a different non-palette area.

Comment on attachment 8899553 [details] Bug 1387313 - Allow dragging non-removable items (url bar, back/forward buttons) within their toolbar. https://reviewboard.mozilla.org/r/170854/#review176292 I would expect this to fail tests, but even if it doesn't, we should add a new test that checks that (a) draggin these items works and (b) dragging them to other areas doesn't work. Sorry I didn't think of this last night when I set the needinfo - my brain must have been too tired. :-( ::: browser/components/customizableui/CustomizeMode.jsm:1571 (Diff revision 1) > item = item.parentNode; > } > > let draggedItem = item.firstChild; > let placeForItem = CustomizableUI.getPlaceForItem(item); > - let isRemovable = placeForItem == "palette" || > + if (item.classList.contains(kPlaceholderClass)) { You can just omit this. The placeholders were for the old panel. Feel free to remove all of the traces of those from this file in a separate commit, or just remove this check altogether. ::: browser/components/customizableui/CustomizeMode.jsm:2215 (Diff revision 1) > return; > } > let doc = aEvent.target.ownerDocument; > doc.documentElement.setAttribute("customizing-movingItem", true); > let item = this._getWrapper(aEvent.target); > - if (item && !item.classList.contains(kPlaceholderClass) && > + if (item && !item.classList.contains(kPlaceholderClass)) { Likewise, this can just be a check for `item` not being null.

Comment on attachment 8899553 [details] Bug 1387313 - Allow dragging non-removable items (url bar, back/forward buttons) within their toolbar. https://reviewboard.mozilla.org/r/170854/#review176712 ::: browser/components/customizableui/test/browser.ini:138 (Diff revision 2) > skip-if = os == "mac" > [browser_1087303_button_preferences.js] > [browser_1089591_still_customizable_after_reset.js] > [browser_1096763_seen_widgets_post_reset.js] > [browser_1161838_inserted_new_default_buttons.js] > +[browser_1387313_allow_dragging_removable_false.js] Nit: please omit the bug number. I've stopped using them because they don't really add anything apart from being able to find the bug that added the test easily - but hg blame lets you do that anyway - and they make autocompleting filenames in this dir a nightmare. ::: browser/components/customizableui/test/browser_1387313_allow_dragging_removable_false.js:3 (Diff revision 2) > +"use strict"; > + > +add_task(async function() { Nit: please add a docstring comment above this task with a 1-sentence explanation of what the test is doing.

Comment on attachment 8899553 [details] Bug 1387313 - Allow dragging non-removable items (url bar, back/forward buttons) within their toolbar. https://reviewboard.mozilla.org/r/170854/#review176716 ::: browser/components/customizableui/test/browser_1387313_allow_dragging_removable_false.js:10 (Diff revision 2) > + let forwardButton = document.getElementById("forward-button"); > + is(forwardButton.getAttribute("removable"), "false", "forward-button should not be removable"); > + ok(CustomizableUI.inDefaultState, "Should start in default state."); > + > + let urlbarContainer = document.getElementById("urlbar-container"); > + let placementsAfterDrag = getAreaWidgetIds(CustomizableUI.AREA_NAVBAR);; Heads-up: eslint chokes on the extra semicolon here.

Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/be0f7ac59f25 Allow dragging non-removable items (url bar, back/forward buttons) within their toolbar. r=Gijs

I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 57.0a1 (2017-08-24), so I'm marking this bug as VERIFIED. Thanks. I found one issue (for now), which is being tracked in bug #1393512.