Most usage is a straight replacement but gtk needs extra changes as it transfers plain text in UTF8 natively and needs to be converted into UTF16, and Windows uses single-byte characters for RTF and CF_HTML formats so we preserve this.
Differential Revision: https://phabricator.services.mozilla.com/D158587
Make Places views constructors arguments more coherent, passing the root
and view elements up to the super class explicitly.
Remove the options argument, that was not strictly necessary, the same info can
be obtained directly.
Rename the "builder" attribute to "afterplacescontent" to clarify what it is
and make panel use it, instead of passing an insertionPoint option.
Additional cleanups:
Make chevron and BMB menus use coherent popupshowing observers.
Remove useless .viewElt accessor, .associatedElement can be used instead.
Avoid an access to the private _rootElt property from the controller.
Differential Revision: https://phabricator.services.mozilla.com/D164827
Only the bookmarks menu button is doing this peculiar re-using of
options.
I tried various fixes: keeping the inheritance but replacing only the
rootElt/viewElt (and using Object.assign to clone the options so modifications
don't 'transmit' to the ancestor menus) but it was messy and net code increase
for a pretty crufty UI surface. I also wasn't sure if that would end up
negatively influencing (now or in the future) e.g. menus shown from the main
bookmarks toolbar view (which would show up as a 'parent' view for the menus
we open from there), and thought that reusing the options was likely to cause
further confusion in future as well, should we add more, uh, options, to them.
So in the end I stuck with Keeping It Simple - I just repeat the one-off
repeating entry class. I'm not even sure how needed this is - in particular,
I wonder why we don't need it for other submenus for 'real' bookmark folders!
But I didn't investigate this too much. If you're sure that we can get rid of
some more of it, happy to do that in a followup.
Note that the addition of _init() was not ultimmately needed to fix this bug
(I think), but it brought the initialization sequence and when we set
`_placesView` more in line with what we did before and what I already did for
`PlacesToolbar`, so that seemed like a good thing to avoid further/other issues.
Differential Revision: https://phabricator.services.mozilla.com/D164756
Unfortunately lazy script getters load the file once for each accessed symbol, which
redeclares "let" and "const" and "class" variables, which was a problem here. But also,
loading the file multiple times is bad for performance (it is not a module so we
actually reload it) and the "lazy" variables get dereferenced immediately from
markup that is present both in browser.xhtml and the hidden window on macOS, so I
doubt lazy loading was getting us anything performance-wise.
This patch simplifies things by 'just' loading the file from the markup.
Differential Revision: https://phabricator.services.mozilla.com/D163934
It was rather complicated because we want the import button to behave
as a bookmark item when in there, but as a regular icon otherwise.
We can achieve that more simply by actually toggling its styling. This
allows us to have less special-cases in toolbarbuttons.css
Differential Revision: https://phabricator.services.mozilla.com/D160045
Long ago, the menu panel in was a customizable area that users could drag things into.
That changed back around 2017 in bug 1354117 when the Photon redesign was built. The
menu panel become a static menu, but we also made it possible to permanently move things
to the overflow panel of the nav-bar.
It looks like we never updated the area type constant from referring to the old menu panel
though, so it's "TYPE_MENU_PANEL", and registering a node for it happens with
registerMenuPanel. This patch changes to constant to TYPE_PANEL and updates the registration
method to registerPanelNode.
I a check around the codebase as well as GitHub looking to see if there were any
system add-ons or experimental WebExtensions that rely on TYPE_MENU_PANEL / registerMenuPanel,
but I couldn't find any.
Differential Revision: https://phabricator.services.mozilla.com/D161078
I have fixed the underlying XPConnect issue, so these workarounds should
no longer be needed.
There are also two more in browser/base/content/browser-siteProtections.js
that I have not fixed.
Differential Revision: https://phabricator.services.mozilla.com/D158158
When these panels had arrows, I guess the bottomcenter topleft alignment
made sense so that you could precisely align the arrow, but that's not
what we do now.
Don't use bottomcenter / leftcenter / rightcenter, since we really want
the sides to align.
This shouldn't change behavior on any platform except Linux + Wayland,
where the alignment looks good now in the case of bug 1784876.
Differential Revision: https://phabricator.services.mozilla.com/D156099
When these panels had arrows, I guess the bottomcenter topleft alignment
made sense so that you could precisely align the arrow, but that's not
what we do now.
Don't use bottomcenter / leftcenter / rightcenter, since we really want
the sides to align.
This shouldn't change behavior on any platform except Linux + Wayland,
where the alignment looks good now in the case of bug 1784876.
Differential Revision: https://phabricator.services.mozilla.com/D156099
The copy guidance recommends adding an ellipsis to the end of labels for
controls that require an additional step to complete the action
specified by the label. We have some menuitems and panel buttons that
open a bookmark dialog or the edit bookmark panel that don't have an
ellipsis, so add that. Also, the form "Bookmark this x" has been changed
to "Bookmark x" in menuitems. Also, there's a "Print Selection" menuitem
that needed an ellipsis. Thanks for the review!
Differential Revision: https://phabricator.services.mozilla.com/D154320
In addition to adding async/await, I also added early return statements
when we know initialization is occuring again, or if there is evidence
that it has been uninitialized.
A concrete example of this would be from the Library, where clicking on
on a folder/category on the left will run initPanel at the same time as
the highlighted folder/bookmark on the right changes (also runs initPanel).
Differential Revision: https://phabricator.services.mozilla.com/D141800
PMDH_onDragEnter sets it but it can be immediately nullified by a dragleave listener of a menu popup. Having null ultimately triggers closeParentMenus() by closeMenuTimer, and setting the proper drop target in dragover prevents that.
Running closeMenuTimer immediately after openen a popup sounds weird but that's an existing behavior and is out of scope of this patch.
Differential Revision: https://phabricator.services.mozilla.com/D127971
The CSS class panel-subview-footer-button was removed from the BMB_bookmarksShowAll
element in the Bookmarks Menu toolbar button, but this class is used by browser-places.js
to know where to insert the user's bookmarks in the structure of the menu.
Since this is the only menu that attachPlacesView attaches to, we can supply
the ID of the bottom Manage Bookmarks element, which will cause browserPlacesView.js
to insert the user's bookmarks above it.
Differential Revision: https://phabricator.services.mozilla.com/D121759
This involves changing moveToAnchor to be allowed while the popup is showing. This change allows the buttons within the tab to use the normal algorithm for determining the tooltip position. This also fixes bug 1695900 so that tooltips for items in bookmarks menus also appear offset as well. Only the main tab and bookmarks on the toolbar appear aligned with the button's bottom edge.
Differential Revision: https://phabricator.services.mozilla.com/D115558
This resolves a failure in browser/base/content/test/pageActions-proton/browser_PageActions_bookmark.js. That test checks the tooltiptext on the bookmark button hbox. Before part 1 of this patch, the hbox always had one of the generic tooltiptexts "Bookmark Current Tab"/"Edit This Bookmark". This tooltiptext was set in updateBookmarkPageMenuItem. Then, in __updateStar, we set the tooltiptext on the <image> contained in the hbox to be "Bookmark this page ([shortcut])"/"Edit this bookmark ([shortcut])". Since the <image> filled the entire hbox, the tooltiptext on the hbox underneath was never seen. Since Part 1 of this patch moved the semantic meaning of the page action buttons from the <image> to the hbox, the tooltiptext containing the shortcut was also moved to the hbox. However, updateBookmarkPageMenuItem was overriding the shortcut tooltiptext with the generic text and thus browser_PageActions_bookmark.js was failing intermittently. The timing of when the tooltiptext was overridden was variable.
This patch stops setting the generic tooltiptext on the hbox altogether. I'm splitting this out into its own patch since I think it needs its own review. With Part 1 applied, when actually using the browser, the hbox had the tooltiptext with the shortcut. I'm not totally clear on why that was working despite updateBookmarkPageMenuItem overriding it, so I'd like to make sure this change is okay.
Depends on D114131
Differential Revision: https://phabricator.services.mozilla.com/D115485
To resolve this bug, we need page action icons and semantic page action nodes to be separate. That way, we can apply filters to the icons without also filtering the nodes' outlines. This means the semantic meaning of the page action button must move up a level, to the enclosing hbox. This means reverting bug 1482025, so I'd like a11y review on this patch.
Differential Revision: https://phabricator.services.mozilla.com/D114131
To resolve this bug, we need page action icons and semantic page action nodes to be separate. That way, we can apply filters to the icons without also filtering the nodes' outlines. This means the semantic meaning of the page action button must move up a level, to the enclosing hbox. This means reverting bug 1482025, so I'd like a11y review on this patch.
Differential Revision: https://phabricator.services.mozilla.com/D114131
