Closed
Bug 808031
Opened 12 years ago
Closed 12 years ago
Double tapping to focus on a text column does not work
Categories
(Core :: General, defect, P3)
Tracking
()
RESOLVED
WORKSFORME
blocking-basecamp | + |
People
(Reporter: st3fan, Assigned: dbaron)
References
()
Details
(Keywords: b2g-testdriver, unagi)
Attachments
(1 file)
(deleted),
image/png
|
Details |
Build 2012-11-01
See screenshot.
To reproduce:
1) Open http://bits.blogs.nytimes.com/2012/11/02/android-nexus-strategy
2) Try to double tap on the main text column to make it zoom to fit.
May be related to zoom factor of the page?
Comment 1•12 years ago
|
||
blocking+ -- per Josh on usability of the browser
blocking-basecamp: ? → +
Component: Gaia → Gaia::Browser
Priority: -- → P3
Blocks: 750977
Component: Gaia::Browser → General
Product: Boot2Gecko → Core
Version: unspecified → Trunk
Reporter | ||
Comment 2•12 years ago
|
||
To clarify: double tap works. The problem is that the positioning and zooming that happens after the double tap is not correct. (Like in my screenshot: the zoomed content is too wide for the display)
Assignee | ||
Comment 3•12 years ago
|
||
I actually don't know where the code for double-tap zoom lives. I thought for Fennec Android it lives in the front end; I'd have guessed the same for B2G given that I don't recall seeing patches to move it into Gecko (though I could certainly have missed something).
Comment 4•12 years ago
|
||
(In reply to David Baron [:dbaron] from comment #3)
> I actually don't know where the code for double-tap zoom lives. I thought
> for Fennec Android it lives in the front end; I'd have guessed the same for
> B2G given that I don't recall seeing patches to move it into Gecko (though I
> could certainly have missed something).
I think the code has landed in https://bugzilla.mozilla.org/show_bug.cgi?id=750977.
The logic is likely http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/AsyncPanZoomController.cpp#574
Adding cjones to the discussion.
The code that decides where to zoom is
http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementScrolling.js#216
AsyncPanZoomController just zooms to whatever rect BrowserElementScrolling tells it to.
Assignee | ||
Comment 6•12 years ago
|
||
So does anyone have a good idea as to who should own this bug?
Assignee | ||
Comment 7•12 years ago
|
||
In other words: if there's an owner for this code who isn't overloaded and wants to look at this, they should go ahead; otherwise either I or sjohnson should probably take it.
Please do!
Comment 9•12 years ago
|
||
Hi David
My patch in bug 805535 may be able to fix it.
Comment 10•12 years ago
|
||
Comment 11•12 years ago
|
||
+cc: ttaubert to try with the same build he's using to test bug 804985
Comment 12•12 years ago
|
||
Bug 804985 doesn't seem to make a difference for me. Works well with and without the patch.
Comment 13•12 years ago
|
||
The attachment 677780 [details] looks like the zoom ratio is very large after double tapping it. And according to my description in bug 804985 and Bug 805535, the large scale number appears when the current zoom is very small and then double tapping.
I guess the reproduce steps:
1. Reproduce Bug 805535 to get the small zoom
1.1 open browser go to any webpage
1.2 find a text area, zoom in to a large scale
1.3 double tap the area
2. double tapping some text area
If the target rect ideal zoom ratio is 5 and after step 1, the scale number should be smaller than 1, assume 0.5 ( On screen area of the webpage area now is 1/4).
At step 2 when double tapping, we will get 10 for the zoom.
10*0.5 = 5; but we set the zoom to 10.
Maybe the case can explain attachment 677780 [details].
Updated•12 years ago
|
Assignee: nobody → dbaron
Comment 14•12 years ago
|
||
One thing I notice that's odd about this code, as compared to mobile/android/chrome/content/browser.js (where the double-tap to zoom code seems to work as expected):
BrowserElementScrolling.js
> 243 let bRect = new Rect(Math.max(cssPageRect.x, rect.x - margin),
> 244 rect.y,
> 245 rect.w + 2 * margin,
> 246 rect.h);
> 247 // constrict the rect to the screen's right edge
> 248 bRect.width = Math.min(bRect.width, cssPageRect.right - bRect.x);
browser.js:
> let bRect = new Rect(Math.max(viewport.cssPageLeft, rect.x - margin),
> rect.y,
> rect.w + 2 * margin,
> rect.h);
> // constrict the rect to the screen's right edge
> bRect.width = Math.min(bRect.width, viewport.cssPageRight - bRect.x);
Why, in BrowserElementScrolling.js, is the left edge specified by cssPageRect.x, but the right edge specified by cssPageRect.right? Perhaps cssPageRect.right is an alias to cssPageRect.x + cssPageRect.width? (I can't seem to find the definition of this data structure, so I'm not entirely sure where it comes from, other than it being set in _recvViewportChange)
Comment 15•12 years ago
|
||
(In reply to Scott Johnson (:jwir3) from comment #14)
> One thing I notice that's odd about this code, as compared to
> mobile/android/chrome/content/browser.js (where the double-tap to zoom code
> seems to work as expected):
>
> BrowserElementScrolling.js
> > 243 let bRect = new Rect(Math.max(cssPageRect.x, rect.x - margin),
> > 244 rect.y,
> > 245 rect.w + 2 * margin,
> > 246 rect.h);
> > 247 // constrict the rect to the screen's right edge
> > 248 bRect.width = Math.min(bRect.width, cssPageRect.right - bRect.x);
>
> browser.js:
> > let bRect = new Rect(Math.max(viewport.cssPageLeft, rect.x - margin),
> > rect.y,
> > rect.w + 2 * margin,
> > rect.h);
> > // constrict the rect to the screen's right edge
> > bRect.width = Math.min(bRect.width, viewport.cssPageRight - bRect.x);
>
> Why, in BrowserElementScrolling.js, is the left edge specified by
> cssPageRect.x, but the right edge specified by cssPageRect.right? Perhaps
> cssPageRect.right is an alias to cssPageRect.x + cssPageRect.width? (I can't
> seem to find the definition of this data structure, so I'm not entirely sure
> where it comes from, other than it being set in _recvViewportChange)
BTW - I'm not suggesting this is the cause of the problem, I'm just curious where the code that will eventually call _recvViewportChange() gets initialized (i.e. where does the viewport data come from?)
Updated•12 years ago
|
We do some awesome printf-JSON-ifying here
http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp#1169
The STR in comment 0 wfm. Please file a new bug if you see new, related symptoms.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•