[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:
  TorBrowserTeam201611R                          |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  SponsorU
-------------------------------------------------+-------------------------

Comment (by arthuredelstein):

 Replying to [comment:48 mcs]:
 > Replying to [comment:47 gk]:
 > > mcs/brade: could you give the new patch a test? I am thinking of
 getting the fix for this ticket into 6.5a4.
 >
 > We did not forget but we were distracted with ESR 45.5 updater changes.
 In any case, we have now tested the latest torbutton patch and it fixes
 the problem we saw.

 That's a relief! Thanks for testing.

 > * The prefs `extensions.torbutton.window.maxWidth` and
 `extensions.torbutton.window.maxHeight` should be removed from torbutton.
 We could include default values for `privacy.window.maxInnerWidth` and
 ``privacy.window.maxInnerHeight` in the browser, but if we do not want to
 encourage people to use them we should leave them out.

 Good point. I have removed them for now.

 > * I am not sure if this is a Firefox bug or not, but it seems as though
 the EXPORTED_SYMBOLS are all available even if you do not include them on
 the left side of a Cu.import() statement. I notice this because you forgot
 to add `observe`. This leads me to think that some of the names in the
 utils.js module are too generic, e.g., `observe` and `showDialog`. In the
 long run we may want to move everything into a container object such as
 `TorButtonUtil`.

 I wasn't aware of this behavior of Cu.import(), but after some research I
 found out that the canonical way to avoid polluting the namespace is to
 add an empty {} as a second argument:

 `let { controller } = Cu.import("resource://torbutton/modules/tor-control-
 port.js", {});`

 Here's a torbutton branch with a couple of followup patches to handle
 these things:
 https://github.com/arthuredelstein/torbutton/commits/19459+4

 > * I don't remember why we limit the window width and height to 95% of
 the available screen dimensions. This leads to surprising behavior such as
 the window being 200 pixels less than it could be when I set
 `privacy.window.maxInnerWidth` to a large number such as 5000. If we just
 want to avoid opening with a full screen window, we could subtract a
 constant number of pixels (e.g, 20) instead of taking away 5% of the
 available space.

 I think the problem is (or was) that some linux flavors incorrectly report
 available width/height, ignoring the presence of a toolbar or the window's
 titlebar. Since we don't really know what these numbers, a percentage
 seemed to make sense. We could also subtract a constant number, but the
 problem is we don't know in advance how many pixels it should be.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19459#comment:49>
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