Closed Bug 459604 Opened 11 years ago Closed 11 years ago

nsDOMWindowUtils::SendMouseEvent doesn't accept points outside the viewport

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: pavlov, Assigned: Gavin)

References

Details

(Keywords: mobile)

Attachments

(1 file, 2 obsolete files)

Go to nytimes.com, should be able to click on links on the page but can't.

I think this is probably a event/content/layout bug.  Needs a reduced (non-fennec) testcase.
Flags: blocking-fennec1.0+
For what it's worth, not calling scrollTo() in _clientToContentCoords() "fixes" the problem, although of course once you've panned any they stop working again, as the events are redispatched to the wrong coordinates. I'm not sure exactly what that means, however.
Looks like the nytimes site now has a "Welcome to TimesPeople" bar at the top of the page which interferes with clicking. We try to scroll the point we're clicking on to 0,0, but the top bar follows the scroll and therefore intercepts the click.

Really what we need is to be able to dispatch events outside of the visible viewport so that we can avoid scrolling entirely.
A somewhat hacky workaround would be to just scroll things to 50,50 instead, or something.
That should be pretty easy. You just want an option to make nsLayoutUtils::GetFrameForPoint (called via PresShell::HandleEvent) build its display list using SetIgnoreScrollFrame on the root scrollframe, like nsPresShell::RenderDocument does when we call it from DrawWindow.
Status: NEW → ASSIGNED
Component: General → DOM
Flags: blocking-fennec1.0+
Product: Fennec → Core
QA Contact: general → general
Summary: Can't click on links on nytimes.com → document.elementFromPoint and nsDOMWindowUtils::SendMouseEvent don't accept points outside the viewport (can't click links on can't click on links on nytimes.com in fennec)
Target Milestone: Fennec A1 → mozilla1.9.1b2
Attached patch patch (obsolete) — Splinter Review
This addresses fennec's use case, by allowing us to click on and find elements outside the viewport. I'm not sure whether the change to PresShell::HandleEvent is going to cause compatibility problems.
Cool, that actually works? :-)

I'm not sure how to expose this in API. We can't just make this change to elementFromPoint, since the spec is clear it should return null for coordinates outside the viewport, but we could add a new method Moz-only method alongside elementFromPoint that handles outside-the-viewport points. We can't just make this change to PresShell::HandleEvent, since it would at least mess up mouse events targeting the viewport scrollbars. We might be able to add a parameter to DOMWindowUtils::SendMouseEvent to trigger this behaviour, but it would be a little ugly since we'd have to add a flag to nsMouseEvent.
Summary: document.elementFromPoint and nsDOMWindowUtils::SendMouseEvent don't accept points outside the viewport (can't click links on can't click on links on nytimes.com in fennec) → document.elementFromPoint and nsDOMWindowUtils::SendMouseEvent don't accept points outside the viewport (can't click links on nytimes.com in fennec)
Attached patch SendMouseEvent patch (obsolete) — Splinter Review
This adds a member to nsGUIEvent about whether or not the scroll frame should be ignored during dispatch. I'm not sure whether this is acceptable, since it bloats all events and is not typically useful, but as you say there aren't really any immediately obvious alternatives for doing this. Other nsIDOMWindowUtils methods could also make use of this, but I only changed sendMouseEvent for now since that's all Fennec cares about.

I'm not really sure what to do with elementFromPoint - I suppose adding an optional argument to its signature isn't an option? If we go with the alternate method route, what interface should it be on, and what should the method be called?
Attachment #343671 - Attachment is obsolete: true
Why can't you add that flag to nsMouseEvent_base? It won't use any more space there. And make it a PRPackedBool.

If you're using elementFromPoint from chrome for something in Fennec, then you could just add your special elementFromPoint variant to nsIDOMWindowUtils.
(In reply to comment #8)
> Why can't you add that flag to nsMouseEvent_base? It won't use any more space
> there. And make it a PRPackedBool.

Ah, I had originally done that, but nsPresShell::HandleEvent takes an nsGUIEvent*, and I hadn't noticed that it was possible to differentiate using eventStructType. I'll do that.
Hmm, should I use eventStructType == NS_MOUSE_EVENT, or NS_IS_MOUSE_EVENT?
I went with eventStructType since that seemed more likely to be reliable.
Attachment #344358 - Attachment is obsolete: true
Attachment #344497 - Flags: superreview?(roc)
Attachment #344497 - Flags: review?(roc)
Comment on attachment 344497 [details] [diff] [review]
SendMouseEvent patch

Are there use cases for other event types (which are also mouse events, or inheriting nsMouseEvent_base). Drag events? Mouse scroll?
Fennec doesn't have an immediate use for non-click mouse events. I'm not sure I understand why you're asking, though.
maybe drag and drop events?
Sure, we might find the need to make additional changes in the future (either to DOMWindowUtils to add the optional parameter to more methods, or to the event classes to allow this for non-mouse events). I don't think we should worry about that until the need arises, though.
Comment on attachment 344497 [details] [diff] [review]
SendMouseEvent patch

+
+  // Optionally ignore scroll frame
+  if (aIgnoreScrollFrame)
+    event.ignoreScrollFrame = PR_TRUE;

Why not just event.ignoreScrollFrame = aIgnoreScrollFrame?

+    if (rootScrollFrame)
+      builder.SetIgnoreScrollFrame(rootScrollFrame);

{} around the statement
Attachment #344497 - Flags: superreview?(roc)
Attachment #344497 - Flags: superreview+
Attachment #344497 - Flags: review?(roc)
Attachment #344497 - Flags: review+
Summary: document.elementFromPoint and nsDOMWindowUtils::SendMouseEvent don't accept points outside the viewport (can't click links on nytimes.com in fennec) → nsDOMWindowUtils::SendMouseEvent doesn't accept points outside the viewport (can't click links on nytimes.com in fennec)
Keywords: mobile
I split off the elementFromPoint part to bug 461803.
Summary: nsDOMWindowUtils::SendMouseEvent doesn't accept points outside the viewport (can't click links on nytimes.com in fennec) → nsDOMWindowUtils::SendMouseEvent doesn't accept points outside the viewport
Landed with those comments addressed:
http://hg.mozilla.org/mozilla-central/rev/7b456bc0eb30
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
I guess this added new warning:
../../../../../dist/include/widget/nsGUIEvent.h:745: warning: ‘nsMouseEvent::clickCount’ will be initialized after
../../../../../../dist/include/widget/nsGUIEvent.h:738: warning:   ‘PRPackedBool nsMouseEvent::ignoreScrollFrame’
../../../../../../dist/include/widget/nsGUIEvent.h:700: warning:   when initialized here
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.