[tor-bugs] #32220 [Applications/Tor Browser]: Change letterboxing color when dark theme is enabled

[Tor Bug Tracker & Wiki]

#32220: Change letterboxing color when dark theme is enabled
-------------------------------------------------+-------------------------
 Reporter:  cypherpunks                          |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:  closed
 Priority:  Medium                               |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:  fixed
 Keywords:  tbb-9.0-issues, tbb-9.0.1-can, ux-   |  Actual Points:  5
  team, TorBrowserTeam201911R                    |
Parent ID:                                       |         Points:  2
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by gk):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Replying to [comment:19 pospeselr]:
 > Ok I've updated the patch with a fixup commit which makes the tabpanel
 color change conditional to letterboxing being enabled, and I've removed
 the redundant addition of "letterboxing" to the browser element class list
 and am now only doing it on one place.
 >
 > tor-browser: https://gitweb.torproject.org/user/richard/tor-
 browser.git/commit/?h=bug_32220_v2&id=b0bd271086e6d727d74d63dddaf13cc73f456ecb

 Thanks this looks good now. The fixup idea is good (and confused me at
 first because I was not used to it from you ;) ) as it makes reviewing
 eas(y)(ier). I squashed it (please do that the next time as this saves
 some time on the reviewer's/merger's side) and applied it to `tor-
 browser-68.2.0esr-9.5` (commit ff8083901a19421e9a3f0dba5346bd6873fee956).

 > And here's a followup/optional commit which adds about:tor to the set of
 pages that do not need letterboxing (like about:blank). It's our built-in
 page, so I don't see any reason why it needs to be letterboxed, but I left
 it out of the fixup commit just in case we do want it letterboxed.
 >
 > tor-browser: https://gitweb.torproject.org/user/richard/tor-
 browser.git/commit/?h=bug_32220_v2&id=733e4014dc0a10c375c70070ec7ee9f468ea73e2

 I am a bit reluctant here and think we should discussing how we want to
 deal with privileged pages and `about:tor` in particular. Here is my
 concern I had:
 {{{
 18:46 <+GeKo> for the about:tor page exemption
 18:47 <+GeKo> i had been thinking about mentioning that in my review
 18:47 <+GeKo> but then thought it might be confusing to users when the
 window is
               suddenly starting "to do" things
 18:47 <+GeKo> while they just tried to visit a page
 18:48 <+GeKo> users don't have the concept of priviledged vs. non-
 priviledged pages
 }}}
 pospeselr: could you open a ticket for that discussion including your idea
 to generally apply letterboxing to privileged sites, too?

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