[tor-bugs] #8478 [TorBrowserButton]: Tor Browser Bundle on OS X 10.6 does not set resolution to a multiple of 200x100

[Tor Bug Tracker & Wiki]

#8478: Tor Browser Bundle on OS X 10.6 does not set resolution to a multiple of
200x100
------------------------------------------------------------------------+---
 Reporter:  cypherpunks                                                 |          Owner:  mikeperry     
     Type:  defect                                                      |         Status:  needs_revision
 Priority:  critical                                                    |      Milestone:                
Component:  TorBrowserButton                                            |        Version:                
 Keywords:  tbb-fingerprinting, tbb-rebase-regression, MikePerry201306  |         Parent:                
   Points:                                                              |   Actualpoints:                
------------------------------------------------------------------------+---
Changes (by gk):

  * status:  needs_review => needs_revision


Comment:

 Replying to [comment:16 mikeperry]:
 > Thanks a lot for this gk. However, I am a little concerned that you've
 switched us from resizing on new window events to the nsIWebProgress
 events with flag STATE_STOP. From your comments, it sounds like you're
 making use of the fact that XUL loading is triggering these progress
 events before content?

 Yes.
 > If so, that might be OK.. But otherwise it makes me a little nervous if
 we're depending on the content to finish loading before resizing.

 We don't. Looking at the URL aProgress.DOMWindow gives us it shows
 chrome://browser/content/browser.xul. My naive interpretation here would
 be that our resizing logic got called as browser.xul (i.e. the chrome) got
 loaded which is way before the update check is started or about:blank is
 loaded.

 > I am also a little sad about adding a listener to listen to *all*
 nsIWebProgress events. If we could avoid the content-related ones somehow,
 that might be better for performance...

 Well, NOTIFY_STATE_ALL is not necessary, indeed. But NOTIFY_STATE_NETWORK
 fires too late (and probably NOTIFY_STATE_REQUEST as well although I have
 not tested that one). Thus, we won't avoid using NOTIFY_STATE_DOCUMENT.
 But that is not too problematic here as testing showed that
 onStateChange()| is called only once after we registered the listener. The
 performance impact should therefore be minimal if existing at all. I'll
 change the patch to use NOTIFY_STATE_DOCUMENT.

 Another idea I got while thinking about the issues you raised: We could
 test the following: Instead of specifying torbutton.xul as the overlay in
 the chrome.manifest we could specify a dummy XUL file which actually does
 not contain any XUL at all but only includes a JS file which loads
 torbutton.xul dynamically (i.e. with |document.loadOverlay()|). This would
 give us the option to register an overlay observer that fires "xul-
 overlay-merged" where we could call our resizing logic. If that works out
 we might avoid patching torbutton.css as the 18x18 icons should already be
 merged before the resizing logic kicks in and don't need webprogress
 listeners. Maybe that approach is preferable for you. :)

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