[tor-bugs] #31251 [Applications/Tor Browser]: Security Level menu hangs off of the toolbarbutton element when it should hang off of the toolbarbutton's child element with toolbarbutton-icon class

[Tor Bug Tracker & Wiki]

#31251: Security Level menu hangs off of the toolbarbutton element when it should
hang off of the toolbarbutton's child element with toolbarbutton-icon class
--------------------------------------+--------------------------------
 Reporter:  pospeselr                 |          Owner:  pospeselr
     Type:  defect                    |         Status:  needs_revision
 Priority:  Medium                    |      Milestone:
Component:  Applications/Tor Browser  |        Version:
 Severity:  Normal                    |     Resolution:
 Keywords:  TorBrowserTeam201907      |  Actual Points:
Parent ID:                            |         Points:
 Reviewer:                            |        Sponsor:
--------------------------------------+--------------------------------
Changes (by acat):

 * keywords:  TorBrowserTeam201907R => TorBrowserTeam201907
 * status:  needs_review => needs_revision


Comment:

 I tested it, and works mostly fine, except when you try to move the button
 via `Customize...`. Then it stops working for the session, but it works
 for new windows and also when restarting. Is it because the patch is now
 caching the button instead of getting it every time?

 Some minor issues:
 {{{
                   classList.add("safest");
 }}}
 Could you remove the trailing spaces?
 Also, there are some empty lines with leading spaces, could you remove
 them too?

 The `get button()` implementation in `browser-places.js` looks slightly
 simpler and also works for me, but up to you to keep this one, looks also
 good (unless this is what is breaking when moving the button around).
 {{{
 openAdvancedSecuritySettings : async function() {
 }}}
 This is not new code, but why `async`? This should be equivalent to the
 same function without `async` and returning `Promise.resolve()`. Is it
 because it's required to return a Promise? Otherwise I think we can remove
 `async` here.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31251#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online