Closed Bug 586288 Opened 15 years ago Closed 14 years ago

Cannot pan XUL, <iframe>s or scrollable <div>s

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 4.0

People

(Reporter: aakashd, Assigned: stechz)

References

Details

(Whiteboard: [Input])

Build Id: Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:2.0b4pre) Gecko/20100811 Namoroka/4.0b4pre Fennec/2.0a1pre and Mozilla/5.0 (Android; U; Linux armv71; en-US; rv:2.0b4pre) Gecko/20100811 Namoroka/4.0b4pre Fennec/2.0a1pre Steps to Reproduce: 1. Go to about:config 2. Try to pan the list Actual Results: Cannot pan the list Expected Results: I should be able to pan the list.
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0b1+
tracking-fennec: 2.0b1+ → 2.0b2+
Summary: Cannot pan No XUL or <iframe>s or scrollable <div>s → Cannot pan XUL, <iframe>s or scrollable <div>s
Depends on: 576192
Assignee: nobody → webapps
Priority: -- → P1
Flags: in-testsuite?
My current strategy to get the bare minimum working: 1) put all iframes in their own container layer, which will be forwarded from the child process to the parent process 2) add an attribute to iframe container layer to indicate it is scrollable 3) add the same attribute to the root content layer 4) add a new scroll function to nsIFrameLoader.idl, that is given both the position that needs scrolling and how much to scroll by. This method will then use the layers tree to figure out what scrollable layer that position belongs to, and will update the offset accordingly.
Step 1 is easy enough, but iframes aren't being drawn in parent process (it looks to be working correctly with non-remote tabs).
Depends on: 600908
Depends on: 602314
Sounds reasonable, but forcing a container layer for each potentially scrollable element will be very bad for performance on some pages.
Yes, I assume we'll have to make that logic a little more sophisticated.
This feels a bit risky for b2. Ben, do you think this will done by Tuesday?
No. I may have patches ready for review this weekend, but I think they may need several iterations. Stuart didn't want to hold b2 back for this, so maybe we should go ahead and put it b3-blocking.
tracking-fennec: 2.0b2+ → 2.0b3+
Depends on: 605618
Depends on: 605630
Modified, more elaborated plan: Part 1 (Bug 605618) 1. Put all iframes in their own container layer and tag them with a unique scrollable ID that crosses the process boundary. 2. Augment display lists for finding cross-process content by allowing a HitTest that returns an ID, that will eventually be redeemable in the content process. 3. Use layers from content process to add shadow iframes to display lists in parent process. 4. API that can do hit test and return ID in parent process on nsFrameLoader. Part 2 (No bug yet) 1. Use hashtable to store ViewportConfigs for nsFrameLoader that are indexable by the scroll ID. 2. Add nsFrameLoader API that, given a scroll ID, can scroll the shadow layer represented by that scroll ID. Part 3 (No bug yet) 1. Only force iframe layers with iframes that have a displayport set. Respect displayport for iframe thebes layers. 2. Add API on content side that will return the proper scrollable element (for now, either the iframe or the root content window) given its unique ID. From here, frontend can either scroll or get the display port. 3. Generalize displayport concept to something that can work with divs. Perhaps add API to nsIScrollableFrame? SetDisplayPort would then likely to the scrollable ID in order to find the proper nsIScrollableFrame.
Using this API, the frontend will be able to control which iframes are retained in memory using displayport, as well as how much to retain for an iframe. The frontend will be responsible for syncing up content position with chrome position for all scrollables, just like frontend has to now for root scroll frame. One problem remains. The frontend needs some way to immediately know if a user is tapping on an iframe, even if it isn't a layer yet. I think we'll need to remote all the geometry for scrollables in the root displayport to nsFrameLoader, even if each iframe does not have its own layer.
If we always create layers for IFRAMEs that have visible scrollbars, then we might be OK performance-wise and you wouldn't have to deal with that last problem. The same might work for DIVs.
So unfortunately maybe there will be a final part that involves generating the shadow display items from stuff that is sent via RenderFrameParent/Child. :( Maybe we just hit test for the display port in the child process, and iterate through finding all scroll frames and serializing their geometry and their transformations?
(In reply to comment #12) > If we always create layers for IFRAMEs that have visible scrollbars, then we > might be OK performance-wise and you wouldn't have to deal with that last > problem. > > The same might work for DIVs. Maybe this would be enough for b3? Furthermore we could not create layers for things outside the displayport. If we could get away with using layers for the display lists, that would be awesome.
We should never create layers for things outside the scrollport under any circumstances.
(In reply to comment #15) > We should never create layers for things outside the scrollport under any > circumstances. OK, that makes sense.
By the way, if we generalize the displayport stuff properly, I think that might mean frontend would have complete control over how many pixels outside the visible viewport is retained for any scrollable region. This might be useful for Fennec's sidebars and URL bars for guaranteeing fast scrolling at the cost of a little bit of memory. We'd want to see if this tradeoff is worth it of course.
(In reply to comment #11) > One problem remains. The frontend needs some way to immediately know if a user > is tapping on an iframe, even if it isn't a layer yet. I think we'll need to > remote all the geometry for scrollables in the root displayport to > nsFrameLoader, even if each iframe does not have its own layer. (In reply to comment #12) > If we always create layers for IFRAMEs that have visible scrollbars, then we > might be OK performance-wise and you wouldn't have to deal with that last > problem. I think the problem here is that the user can pan arbitrarily far before we get an updated layer tree from the content process. This means the user can tap outside content's displayport (layer tree). I think we should return some "UNKNOWN" value from a chrome-process hit test and then pay the round-trip cost to content on UNKNOWN (if there are scrollable sub-frames on the page).
(In reply to comment #14) > Maybe this would be enough for b3? Yes, let's do it!
(In reply to comment #19) > I think the problem here is that the user can pan arbitrarily far before we get > an updated layer tree from the content process. This means the user can tap > outside content's displayport (layer tree). I think we should return some > "UNKNOWN" value from a chrome-process hit test and then pay the round-trip cost > to content on UNKNOWN (if there are scrollable sub-frames on the page). I presume that once you start scrolling we start requesting new layer trees, so if the user gets into an unknown region we should already have requested a new layer tree for that region on the way automatically. We should just assume there's nothing there in the unknown region, until the new layers arrive.
(In reply to comment #19) > I think we should return some > "UNKNOWN" value from a chrome-process hit test and then pay the round-trip cost > to content on UNKNOWN (if there are scrollable sub-frames on the page). Hmm, or we could just drop the tap, or queue it, or whatever else. At any rate, I'm not sure this case merits remoting the frame tree.
If the display port has not come in yet for a visible region, the user is probably expecting the root scroll pane to scroll IMO.
How is the progress on this bug? Do we have any patches or WIP?
See scroll iframes bug. Patches are in the process of code review. I'm not sure div panning is going to make it. I don't have any patches for it yet and it will certainly depend on the work done in the iframe panning bug.
Scroll iframes bug is in depends on list: bug 605618
this looks like a meta bug, so we're un-blocking it
tracking-fennec: 2.0b3+ → ---
(In reply to comment #31) > this looks like a meta bug, so we're un-blocking it But shouldn't all bugs this bug depends on then be blocking fennec b3+? Bug 605630 is not blocking.
(In reply to comment #33) > Bug 605630 won't block b3 Ah okay, I understand. What exactly will work, when 602314 and 605618 are fixed? iframe and div panning?
(In reply to comment #34) > (In reply to comment #33) > > Bug 605630 won't block b3 > > Ah okay, I understand. What exactly will work, when 602314 and 605618 are > fixed? iframe and div panning? Only iframes to start. div overflow and XUL will follow later, building on the initial work.
Blocks: 618975
No longer blocks: 618975
Depends on: 618975
I think this can be closed.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
VERIFIED FIXED on Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:5.0a2) Gecko/20110413 Firefox/5.0a2 Fennec /5.0a2 Device: Motorola Droid 2 (Android 2.2), HTC Desire Z (Android 2.2)
Status: RESOLVED → VERIFIED
Target Milestone: --- → Firefox 4.0
You need to log in before you can comment on or make changes to this bug.