Closed Bug 1388029 Opened 6 years ago Closed 6 years ago

Remove remaining mentions of AREA_PANEL from the tree

Categories

(Firefox :: Toolbars and Customization, enhancement, P1)

55 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox57 --- wontfix
firefox59 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

(Whiteboard: [reserve-photon-structure])

Attachments

(5 files)

Bug 1354117 removed a bunch of this, but there are still some mentions left. The old XUL is also still there, as well as a bunch of now-outdated CSS.

Note that a number of panelviews will need to be put somewhere else rather than removed, as they are still in use in the new hamburger panel and/or for extant toolbar widgets.
Flags: qe-verify-
Priority: -- → P3
Whiteboard: [reserve-photon-structure]
Priority: P3 → P4
Priority: P4 → P5
Priority: P5 → P3
Bug 1410666 removed the PanelUI code and CSS, but it didn't remove all instances of AREA_PANEL.
Summary: Remove remaining mentions of AREA_PANEL, remove PanelUI-popup/contents, and any associated CSS, from the tree → Remove remaining mentions of AREA_PANEL from the tree
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> Bug 1410666 removed the PanelUI code and CSS, but it didn't remove all
> instances of AREA_PANEL.

There's also a bunch of .panelUI-grid CSS that still needs to die, and possibly more...
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment on attachment 8925884 [details]
Bug 1388029 - remove useless param from PanelUI.showSubView,

https://reviewboard.mozilla.org/r/197104/#review202354
Attachment #8925884 - Flags: review?(jaws) → review+
Comment on attachment 8925885 [details]
Bug 1388029 - remove notion of 'wide' widget and panel columns from the tree,

https://reviewboard.mozilla.org/r/197106/#review202356

::: browser/components/customizableui/DragPositionManager.jsm:16
(Diff revision 1)
>  const kPaletteId = "customization-palette";
> -const kPlaceholderClass = "panel-customization-placeholder";
>  
>  this.EXPORTED_SYMBOLS = ["DragPositionManager"];
>  
>  function AreaPositionManager(aContainer) {

Now that our menu is only displayed in one direction I bet we can make this file even simpler. We don't need to do anything about width in here.
Attachment #8925885 - Flags: review?(jaws) → review+
Comment on attachment 8925886 [details]
Bug 1388029 - remove the remaining uses of AREA_PANEL,

https://reviewboard.mozilla.org/r/197108/#review202358
Attachment #8925886 - Flags: review?(jaws) → review+
Comment on attachment 8925887 [details]
Bug 1388029 - remove dead CSS relating to the old panel,

https://reviewboard.mozilla.org/r/197110/#review202360
Attachment #8925887 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Comment on attachment 8925885 [details]
> Bug 1388029 - remove notion of 'wide' widget and panel columns from the tree,
> 
> https://reviewboard.mozilla.org/r/197106/#review202356
> 
> ::: browser/components/customizableui/DragPositionManager.jsm:16
> (Diff revision 1)
> >  const kPaletteId = "customization-palette";
> > -const kPlaceholderClass = "panel-customization-placeholder";
> >  
> >  this.EXPORTED_SYMBOLS = ["DragPositionManager"];
> >  
> >  function AreaPositionManager(aContainer) {
> 
> Now that our menu is only displayed in one direction I bet we can make this
> file even simpler. We don't need to do anything about width in here.

We don't use this thing for the menu anymore, but we still use it for the palette, where it is somewhat broken (bug 1395950, which doesn't explicitly mention it but we no longer transition things as we used to... I don't really know how/when this broke, haven't had time to investigate, and seemed much less important than a lot of other stuff, also because we might change how palette dnd works).
Looks like this upsets browser/base/content/test/touch/browser_menu_touch.js on Windows 10, and I'm away from my win10 machine so I can't check why that is. :-(
Comment on attachment 8927820 [details]
Bug 1388029 - make test messages of browser_menu_touch.js more descriptive,

https://reviewboard.mozilla.org/r/199118/#review204130

Looks good to me, thank you. This gets a little awkward once one of the elements doesn't have an id, but I guess we'll let whoever runs into that worry about it.
Attachment #8927820 - Flags: review?(jhofmann) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/db290bb0e493
remove useless param from PanelUI.showSubView, r=jaws
https://hg.mozilla.org/integration/autoland/rev/a4bd6d4cb58b
remove notion of 'wide' widget and panel columns from the tree, r=jaws
https://hg.mozilla.org/integration/autoland/rev/cc79288915ef
remove the remaining uses of AREA_PANEL, r=jaws
https://hg.mozilla.org/integration/autoland/rev/f15df5a149ac
remove dead CSS relating to the old panel, r=jaws
https://hg.mozilla.org/integration/autoland/rev/287dfb07b9f9
make test messages of browser_menu_touch.js more descriptive, r=johannh
The issue that caused test failure was that I'd missed this callsite:

https://dxr.mozilla.org/mozilla-central/source/browser/components/customizableui/CustomizableUI.jsm#1633

of showSubView in the first patch.
You need to log in before you can comment on or make changes to this bug.