497347 - Can't zoom an element nested into an <iframe />

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

If an element is nested into an <iframe /> and we want to zoom this element, zoomToElement actually use the iframe as a target.

Comment 1

[Ben Combee (:bcombee)]

Fixes for iframe scrolling should also have the code changes needed to fix this
due to change in elementFromPoint behavior.

Comment 2

[Ben Combee (:bcombee)]

My iframe patch doesn't quite fix this, not sure why yet.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Ben, 
I've just tried the last version of your iframe patch, elementFromPoint return the right element for me, but, the problem come from zoomToElement (in CanvasBrowser) which seems to not properly handle nested element.

Comment 4

[Ben Combee (:bcombee)]

thanks for the test.  zoomToElement is part of CanvasBrowser and is being replaced in the Tilecache code; assuming that code makes it over in roughly the right shape, _getPagePosition needs to be fixed... it should verify that the element is part of the browser, and if not, do a recursive find position, check parent loop.  If it finds the browser, great, otherwise abort because the element isn't in the tree.

Comment 5

[Ben Combee (:bcombee)]

I'm going to go ahead and try to fix this on current trunk, with the understanding that the code will need to be changed once tilecache lands.  We won't land this fix until then.

Comment 6

[Ben Combee (:bcombee)]

Reinvestigating!

Comment 7

[Benjamin Stover (:stechz)]

Attachment: Fix to elementFromPoint

Zooming into iframes was broken if the document's scroll offset was changed (like from a hyperlink).  elementFromPoint should not have been adding iframe scroll offsets.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 399243 [details] [diff] [review]
Fix to elementFromPoint


>diff -r a372c277c2a3 -r ef2cb7737ea1 chrome/content/browser.js

>    * return element at client coordinates of browser, returns null if
>    * there's no active browser or if no element can be found
>    */
>-  elementFromPoint: function elementFromPoint(x, y) {
>+  elementFromPoint: function elementFromPoint(/* browser coords */ x, y) {

Can you put a comment in the above comment block and not in the function arguments?

>+      // adjust client coordinates' origin to be top left of iframe viewport
>       var rect = elem.getBoundingClientRect();

Change to 'let' cause 'let' is the new 'var'

Comment 9

[Benjamin Stover (:stechz)]

Attachment: Code review changes

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
https://hg.mozilla.org/mobile-browser/rev/d54721fa98c2