[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #19459 [Applications/Tor Browser]: Write (C++) patch for window resizing parts



#19459: Write (C++) patch for window resizing parts
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:
                                                 |  arthuredelstein
     Type:  task                                 |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-torbutton-conversion,            |  Actual Points:
  TorBrowserTeam201610R                          |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  SponsorU
-------------------------------------------------+-------------------------

Comment (by mcs):

 Kathy and I reviewed both patches. Here are a few comments on the
 Torbutton patch:
 * In `torbutton_resizelistener.onStateChange`, the container variable can
 be removed.
 * The `extensions.torbutton.startup_resize_period` pref (aka
 `k_tb_tor_resize_warn_pref`) is never set to false and its value is never
 read, so it seems like it can be removed too.
 * Since the call to `quantizeBrowserSize()` was removed, should the entire
 `src/chrome/content/content-sizer.js` file be removed? Or will that
 functionality be reinstated?

 And here are our comments on the browser (C++) patch:
 * Please add a brief comment above
 `nsXULWindow::ResizeToRoundedDimensions()` to explain what it does (1000 x
 1000 maximum, width constrained to a multiple of 200, etc.)
 * `nsXULWindow::ResizeToRoundedDimensions()` should have `return NS_OK` at
 the end.
 * Are we sure that it is okay to remove support for the
 `extensions.torbutton.window.innerWidth` / `innerHeight` prefs? With the
 new patches, there is no way for the user to override the initial window
 size unless they disable fingerprinting protection. That might be okay,
 but I think people who have set these prefs in the past will be surprised.

 A related issue (but not really for this ticket) is that constraining the
 width to a multiple of 200 might be annoying on Android devices that have
 narrow screens (old phones)?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19459#comment:36>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs