Closed Bug 770847 Opened 12 years ago Closed 12 years ago

[BrowserAPI] mozbrowserscroll event to inform embedder when the content has scrolled

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: benfrancis, Assigned: vingtetun)

References

Details

Attachments

(1 file)

We need a new event on mozbrowser iframes which fires when the user scrolls the page, which reports scrollTop (and Dale suggests maybe scrollLeft as well for completeness).

The first use case for this event is to allow the Gaia browser's address bar to be scrolled on/off the screen as the user scrolls the content.
Attached patch PatchSplinter Review
Justin do you know if reading window.scrollX/scrollY force a reflow? I think it doesn't but I want to be sure.

Also I'm not sure this feature alone resolve the use case described by Ben.

To be able to show/hide the titlebar of the browser application you need to have this information *and* the scrolling values not consumed by the content.
Attachment #639954 - Flags: review?(justin.lebar+bug)
> Justin do you know if reading window.scrollX/scrollY force a reflow? I think it doesn't but I want 
> to be sure.

I don't, but bz should.

> To be able to show/hide the titlebar of the browser application you need to have this information 
> *and* the scrolling values not consumed by the content.

IOW, you need this and bug 757859, right?
Comment on attachment 639954 [details] [diff] [review]
Patch

It looks like reading scroll{X,Y} can cause us to flush invalidations on the parent window (nsGlobalWindow::EnsureSizeUpToDate()).  I don't know if that's the same thing as reflow.  But in any case, OOP browser tabs won't have a parent window.

It also /may/ cause us to flush invalidations on the window itself, but it tries not to do so.

So I /think/ we're OK, but I'd like bz to confirm.  Code looks great, thanks.
Attachment #639954 - Flags: review?(justin.lebar+bug)
Attachment #639954 - Flags: review+
Attachment #639954 - Flags: feedback?(bzbarsky)
Currently in Gecko reading window.scrollX/scrollY will always flush reflow on the parent, if there is a parent.  This is because changing the size of a window can affect its scrollX/scrollY, so we have to make sure that anyone who can both change our size and read our scroll position gets consistent answers.

In the common case, I would expect a reflow flush on the parent of a mozbrowser to be cheap.

But somewhat worse for your situation is that any time the window is not scrolled all the way to the top-left corner of the document getting scrollX/Y will _also_ flush layout on the window itself.  The reason is documented in nsGlobalWindow::GetScrollXY, but basically the scroll position depends on the layout of the content in the window.

For your use case, would you want scroll positions that are in an inconsistent state because there are pending reflows that just haven't happened yet?
(In reply to Justin Lebar [:jlebar] from comment #2)
> > To be able to show/hide the titlebar of the browser application you need to have this information 
> > *and* the scrolling values not consumed by the content.
> 
> IOW, you need this and bug 757859, right?

I think something else is needed in order to achieve what they want.

Since you don't have access to the mouse events of the inner window you don't know what is the distance of the movement, or even you don't know if there is a pan event.

This lead to a few weird things like:
 - if you listen scroll event, a webpage will be able to hide the urlbar by scrolling it's content while you likely want to hide the url bar only on user gesture.

 - If your content is at 0,0 and the user pan 100px to the top the url bar should go way before panning anything but with this solution the content will be scrolled first.

 - since you don't know the distance of the pan gesture this is hard to determine how many pixels of your urlbar you want to scroll.
Summary: [BrowserAPI] mozbrowsercroll event to inform embedder when the content has scrolled → [BrowserAPI] mozbrowserscroll event to inform embedder when the content has scrolled
(In reply to Boris Zbarsky (:bz) from comment #4)
> 
> For your use case, would you want scroll positions that are in an
> inconsistent state because there are pending reflows that just haven't
> happened yet?

I don't think they want. One could argue that it will be possible to rectify the values by listening window sizes changes (bug 757859) but this seems a workaround.


So I wonder if the solution to the described use case is not something completely different. For example if one embedder want to add gestures support it should be an API that help us for that. And imho what they want to do if a bit similar.

If what I said is correct I think the timeline is too short for the release so I will postpone the feature if that's possible?
> If what I said is correct I think the timeline is too short for the release so I will postpone the 
> feature if that's possible?

This sounds pretty hard to get right, yes!
Comment on attachment 639954 [details] [diff] [review]
Patch

Anyway, feedback is above.  ;)
Attachment #639954 - Flags: feedback?(bzbarsky)
Vivien, it sounds like this is blocking some browser front-end work.  Do you want to push this patch through (and/or figure out what needs to be done), or do you want to hand it off to someone else?
> This lead to a few weird things like:
> - if you listen scroll event, a webpage will be able to hide the urlbar by 
> scrolling it's content while you likely want to hide the url bar only 
> on user gesture.

I just wanted to mention that a lot of web content will scroll automatically just to get rid of the url bar (on ios / android), hiding the url bar is a really annoying interaction problem

http://remysharp.com/2010/08/05/doing-it-right-skipping-the-iphone-url-bar/
This is actually a security issue because web pages would be able to spoof the URL bar.
(In reply to Justin Lebar [:jlebar] from comment #9)
> Vivien, it sounds like this is blocking some browser front-end work.  Do you
> want to push this patch through (and/or figure out what needs to be done),
> or do you want to hand it off to someone else?

I'm just not sure of how useful is this patch. I can land it if needed but I don't think it will resolve anything :/
> I'm just not sure of how useful is this patch.

Based on comment 5, or something else?  Here was my suggestion in github: https://github.com/mozilla-b2g/gaia/issues/1222#issuecomment-6849060

We need to do /something/ about hiding the URL bar in the browser (where "something" might include "deciding we're not going to do anything")...
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd4e6a53400d
Assignee: nobody → 21
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla17
I thought we'd decided this wasn't useful?  What are you going to use this for, given bug 774809?
(In reply to Justin Lebar [:jlebar] from comment #15)
> I thought we'd decided this wasn't useful?  What are you going to use this
> for, given bug 774809?

I though it is still needed to know the current scrolling position of the browser. If it is at 0,0 then the urlbar can be scrolled if there is a gesture. Otherwise it should not. Or maybe I've missed a way to get this information with a different API ?
Ah, we realized on IRC that you need to know the current scroll position so that when you're scrolling up, we unhide the URL bar only once you've scrolled to the top of the page.
https://hg.mozilla.org/mozilla-central/rev/cd4e6a53400d
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: