Last Comment Bug 712472 - click-and-drag in Tilt becomes wonky after you do full-page-zoom (Ctrl +)
: click-and-drag in Tilt becomes wonky after you do full-page-zoom (Ctrl +)
Status: RESOLVED FIXED
[tilt]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: Firefox 12
Assigned To: Victor Porof [:vporof][:vp]
:
Mentors:
http://ftp.mozilla.org/pub/mozilla.or...
Depends on: 712286
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-20 15:33 PST by Daniel Holbert [:dholbert]
Modified: 2012-01-03 05:46 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
screencast (3.75 MB, video/ogg)
2011-12-20 16:00 PST, Daniel Holbert [:dholbert]
no flags Details
v1 (5.13 KB, patch)
2011-12-21 04:42 PST, Victor Porof [:vporof][:vp]
no flags Details | Diff | Splinter Review
v2 (11.73 KB, patch)
2011-12-22 05:27 PST, Victor Porof [:vporof][:vp]
no flags Details | Diff | Splinter Review
v3 (15.36 KB, patch)
2011-12-23 04:39 PST, Victor Porof [:vporof][:vp]
no flags Details | Diff | Splinter Review
v4 (15.41 KB, patch)
2011-12-23 08:26 PST, Victor Porof [:vporof][:vp]
rcampbell: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2011-12-20 15:33:10 PST
STR:
 1. visit http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ (or any site, really)

 2. Right-click on the upper blob of text, hit "Q" for inspect, and then Alt-M for 3D

 3. Hold Ctrl (or Cmd on mac) and mash your "+" key a few times to do full-page-zoom
    Do this until you can't see the gray background anymore.

 4. Click the top-center of the page and drag it towards the bottom of the screen.

EXPECTED RESULTS:  Page tips forward, "bowing" to me.
ACTUAL RESULTS:    Page rotates to the right

I'm guessing that the coordinates of the click position aren't getting scaled by the fullpage zoom factor, or something, so a click at the top-center is interpreted as a click at the top-right.
Comment 1 Daniel Holbert [:dholbert] 2011-12-20 15:33:40 PST
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111219 Firefox/11.0a1
Comment 2 Daniel Holbert [:dholbert] 2011-12-20 16:00:17 PST
Created attachment 583327 [details]
screencast

screencast of normal behavior followed by broken-behavior-after-fullpage-zoom
Comment 3 Victor Porof [:vporof][:vp] 2011-12-21 00:05:23 PST
Funny.
Zooming shouldn't be supported yet, bug 710762 adds that functionality (without the need of using Ctrl/Cmd). Right now, Ctrl+zooming actually scales the entire canvas, instead of translating in the 3D environment. This should be avoided.
Comment 4 Victor Porof [:vporof][:vp] 2011-12-21 04:42:09 PST
Created attachment 583456 [details] [diff] [review]
v1
Comment 5 Rob Campbell [:rc] (:robcee) 2011-12-21 05:29:42 PST
Comment on attachment 583456 [details] [diff] [review]
v1

test please!
Comment 6 Victor Porof [:vporof][:vp] 2011-12-22 05:27:40 PST
Created attachment 583763 [details] [diff] [review]
v2
Comment 7 Victor Porof [:vporof][:vp] 2011-12-22 07:12:33 PST
Comment on attachment 583763 [details] [diff] [review]
v2

On some Linux machines:

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/tilt/test/browser_tilt_zoom.js | The renderer width wasn't set correctly.
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/tilt/test/browser_tilt_zoom.js | The renderer height wasn't set correctly.

Investigating...
Comment 8 Victor Porof [:vporof][:vp] 2011-12-23 04:39:55 PST
Created attachment 584035 [details] [diff] [review]
v3

Fixed test failures. Try is green.
https://tbpl.mozilla.org/?tree=Try&rev=52a708a07be0
Comment 9 Victor Porof [:vporof][:vp] 2011-12-23 05:12:56 PST
There seems to be one Win intermittent orange in a webconsole test. There's bug 676696 filed for that, but hasn't occurred since 2011-08-29, so I'm not sure if it's related to this patch or not.
Comment 10 Victor Porof [:vporof][:vp] 2011-12-23 08:26:40 PST
Created attachment 584071 [details] [diff] [review]
v4

The orange may (may!) be caused by resizing the browser window. Doing another test run with a new patch that reverts the size to original after the tests are finished seems to avoid causing the intermittent orange, but we'll never know. For safety, I added the "improved" patch.

For the record, I remember also triggering this orange a while ago by not doing a removeChild for a canvas added to the selectedBrowser.parentNode. This remained stuck in the xul window, and thus altered the innerWidth and innerHeight of the content document when the webconsole tests were run. Maybe this is the underlying reason for bug 676696?
Comment 11 Rob Campbell [:rc] (:robcee) 2011-12-23 09:37:44 PST
Comment on attachment 584071 [details] [diff] [review]
v4

+TiltUtils.getDocumentZoom = function TU_getDocumentZoom() {
+  let browserWindow = Cc["@mozilla.org/appshell/window-mediator;1"]
+    .getService(Ci.nsIWindowMediator)
+    .getMostRecentWindow("navigator:browser");
+
+  return browserWindow.gBrowser.selectedBrowser.markupDocumentViewer.fullZoom;
+};

this feels like it should be a utility method in the InspectorUI object since we seem to keep re-implementing it. Maybe even a shared utils method.

A lot of TiltUtils.jsm looks like it could be refactored to live in shared/. We should keep that in mind.

wow, totally missed those bad indents in browser_tilt_controller.js. Thanks for fixing.
Comment 12 Victor Porof [:vporof][:vp] 2011-12-23 09:56:13 PST
(In reply to Rob Campbell [:rc] (robcee) from comment #11)
> Comment on attachment 584071 [details] [diff] [review]
> v4
> 
> wow, totally missed those bad indents in browser_tilt_controller.js. Thanks
> for fixing.

What bad indents? No bad indents, I just wrapped a lot of code in function testEventCancel(cancellingEvent), to reuse that stuff a bit later in the test :)
Comment 14 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-01-03 05:46:44 PST
https://hg.mozilla.org/mozilla-central/rev/8be4e3d2105e

Note You need to log in before you can comment on or make changes to this bug.