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

RESOLVED FIXED in fennec1.0b4

Status

Fennec Graveyard
Panning/Zooming
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: vingtetun, Assigned: stechz)

Tracking

Trunk
fennec1.0b4

Details

Attachments

(1 attachment, 1 obsolete attachment)

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

Updated

9 years ago
tracking-fennec: --- → ?
Fixes for iframe scrolling should also have the code changes needed to fix this
due to change in elementFromPoint behavior.
Depends on: 458741
My iframe patch doesn't quite fix this, not sure why yet.
tracking-fennec: ? → 1.0+
Assignee: nobody → combee
tracking-fennec: 1.0+ → 1.0b3+
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.
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.
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.
Reinvestigating!
(Assignee)

Comment 7

8 years ago
Created attachment 399243 [details] [diff] [review]
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.
Assignee: combee → webapps
Status: NEW → ASSIGNED
Attachment #399243 - Flags: review?(mark.finkle)
(Assignee)

Updated

8 years ago
Attachment #399243 - Flags: review?(combee)
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'
Attachment #399243 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 9

8 years ago
Created attachment 399301 [details] [diff] [review]
Code review changes
Attachment #399301 - Flags: review?(mark.finkle)
(Assignee)

Updated

8 years ago
Attachment #399301 - Flags: review?(combee)
(Assignee)

Updated

8 years ago
Attachment #399243 - Attachment is obsolete: true
Attachment #399243 - Flags: review?(combee)
Attachment #399301 - Flags: review?(mark.finkle) → review+

Updated

8 years ago
Attachment #399301 - Flags: review?(combee) → review+
pushed:
https://hg.mozilla.org/mobile-browser/rev/d54721fa98c2
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → B4
Component: General → Panning/Zooming
You need to log in before you can comment on or make changes to this bug.