Closed Bug 703573 Opened 8 years ago Closed 8 years ago
HTML iframe cannot be panned
Mozilla/5.0 (Android;Linux armv7l;rv:11.0a1)Gecko/20111118 Firefox/11.0a1 Fennec/11.0a1 Devices: Samsung Galaxy Nexus S OS: Android 2.3.4 Steps to reproduce: 1. Open Fennec App 2. Browse to http://www.htmlcodetutorial.com/frames/_IFRAME.html 3. Pan the iframe content Expected result: The content of the iframe can be panned. Actual result: The content of the iframe cannot be panned
blocking bug 695448?
Assignee: nobody → chrislord.net
Really don't know the best way to fix this with our current Java compositor scheme. One thing we can do; Get browser.js to inform us of scrollable sub-frames (and their extents) as part of the drawing metadata it provides us, then pass through events in those areas and let browser.js handle scrolling. I'm not sure how sucky this would feel - it would probably mean sub-frames lag terribly during loading/on intensive pages/when zoomed, etc. and I can't imagine the update performance would be great either (it'd need to upload the region of the sub-frame for every position change) - certainly until we eliminate the required texture upload. Another thing; We could just disable overflow and not handle iframes - this is what early iOS/Android did. Not many sites use scrollable iframes, or rely that much on the behaviour of overflow. Whether this is acceptable now that all our competition handle this correctly though, is another question. I would say it isn't, but it's not like the alternative is so great either... I did bring this up when we discussed the idea of a Java compositor, but apparently it wasn't an issue then? Maybe pcwalton has more ideas in this area.
I think GMail and Google Reader both use scrollable iframes, at least in the versions that are served to Fennec.
Yeah, I figured we'd do one of those two things. iOS (even now) is really bad at subdocument frames too, so we aren't alone here. The Gmail and Google Reader is unfortunate. We should have some kind of solution here.
(In reply to Patrick Walton (:pcwalton) from comment #4) > Yeah, I figured we'd do one of those two things. iOS (even now) is really > bad at subdocument frames too, so we aren't alone here. Not sure why you say that. iOS since ages uses a feature called frame-flatten, which makes an inner frame viewport as big as its contents size, making it non-scrollable. It scrolls together with the main frame as one canvas. It breaks some sites, but eliminates the in-region scrolling overhead. I dislike it, personally. They now also do a nice work on scrollable divs with iOS5. See http://functionsource.com/post/scrollability-can-hopefully-become-a-shim
I'd like to go with my first suggestion of how to handle this. We'll handle the events in the Java and browser.js will have a signal it can send to say "I'm handling viewport updates" when a touch event occurs. In this situation, Java will then continue to process panning/flinging, but instead of updating the viewport, it'll send the events to JS to handle. This will stop us from having multiple versions of the kinetic panning code lying around. When browser.js sends this signal, Java will undo any scrolling its done (which shouldn't happen often, as there's a drag threshold and Gecko should be able to respond quite quickly, other than during page-load) and hand-off to browser.js to do its thing. I'd have hoped that sub-frames are usually smaller than the viewport, and we've already shown that Gecko can update at 60Hz in a lot of cases without async scrolling, so this shouldn't be too bad (as long as we can minimise/remove the texture-upload overhead). I'd like this to depend on bug #697701, as it has code that will make this an easier change to implement.
Depends on: 697701
This patch adds scrolling support in the way I described in comment #6. It doesn't do the 'undo scroll' step, I'm not sure if we really want that or not (and it can be fixed in another bug if we do).
Attachment #578273 - Flags: review?(kgupta) → review-
Hopefully addressed all comments. I'm not particularly happy with the readability of this code, but hopefully bug #701594 will tidy things up a bit, and then perhaps some follow-up to separate the gesture detection from the actual panning/zooming code.
Comment on attachment 579079 [details] [diff] [review] Add scrolling support for document sub-frames (revised) From kats' review on pastebin: 1. mOverridePanningAck could be renamed to mOverrideScrollAck, similarly mOverridePanningPending could be mOverrideScrollPending 2. viewportPos in Axis should be renamed mViewportPos since it's private now - float excess = getExcess(); - if (FloatUtils.fuzzyEquals(excess, 0.0f)) - setFlingState(FlingStates.STOPPED); - else + setFlingState(FlingStates.STOPPED); + if (!disableSnap && !FloatUtils.fuzzyEquals(getExcess(), 0.0f)) setFlingState(FlingStates.WAITING_TO_SNAP); 3. This condition should be inverted to "if (disableSnap || FloatUtils....)" for clarity and consistency with the other hunk later.
Attachment #579079 - Flags: review?(kgupta) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Testing 20111207 nightly, I can confirm that this LinkedIn article using an iframe scrolls slightly better, but still has some problems: http://www.linkedin.com/news?viewArticle=&articleID=952157269&gid=3853987&type=member&item=83091217&articleURL=http%3A%2F%2Fengineering.linkedin.com%2Fnodejs%2Fblazing-fast-nodejs-10-performance-tips-linkedin-mobile&urlhash=s_P3&goback=.gde_3853987_member_83091217&_mSplash=1 Flicking over the main content area quickly still usually pans the entire page, while slowly tapping-and-dragging (sslloowwllyy) pans the iframe.
No longer depends on: 709437
Duplicate of this bug: 709437
Environment: Aurora 2012-01-19 Firefox/11.0a2 Fennec/11.0a (2012-01-20) Nightly 2012-01-19 Firefox/12.0a1 Fennec/12.0a (2012-01-20) Device:HTC Desire Z Android(2.3.3) Bug is still reproducible on Aurora and Nightly as originally described in bug description. Reopening bug
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
wfm today's nightly.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → WORKSFORME
This is NOT working for me in Firefox 21.0 for Android.
(In reply to Shane Tomlinson [:stomlinson] from comment #17) > This is NOT working for me in Firefox 21.0 for Android. This code has basically been entirely rewritten - the original bug was over a year ago :) If you have issues with iframes and reasonable STR, please open a new bug and cc me and :kats
Depends on: 911574
You need to log in before you can comment on or make changes to this bug.