Closed Bug 517737 Opened 10 years ago Closed 10 years ago

dragging of a route in Google Maps doesn't work properly

Categories

(Core :: User events and focus handling, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
blocking2.0 --- beta1+

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

(Keywords: regression)

Attachments

(3 files, 3 obsolete files)

Steps to reproduce: create a route from some location to another location in google maps. Click on the route somewhere and try to drag it to another path. The path doesn't update properly or at all.

Regression range is http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bf0fdec8f43b&tochange=912c6ae3b70c

From that bug 503943 seems like the most obvious candidate.
setCapture(area) is being done here, but capturing on an <area> doesn't seem to be the problem.

Instead, it looks that svg elements are used here for something. Dragging over the directional path itself targets an <img> and works fine, but dragging outside the path targets an <svg> element.
Sorry, I should say that the little white dot is the <img> but everywhere else is an <svg> element.
This is a pretty serious regression to a popular site and a very useful feature. Can we please not ship this in 3.6?
Flags: blocking1.9.2?
Are you seeing this on 3.6? I don't and from the looks of the bug that caused this regression (bug 503943), it isn't on 1.9.2 and there are no plans for it to be.
OK. If this is trunk only, then I guess we don't have to worry, yet. Though it still sucks that the regression is there at all.
Flags: blocking1.9.2?
blocking2.0: --- → ?
A testcase would be good here as the maps code is quite complicated.
Keywords: testcase-wanted
I suspect that what is happening is that some script is checking if element.setCapture exists and adjusting what happens to assume that IE is being used, thus failing.
OS: Windows 2000 → All
Hardware: x86 → All
setting the useragent on trunk to Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 ID:20091121045457 doesn't fix it.
Keywords: dogfood
Attached file testcase
I think this testcase represents the problem, but I am not totally sure of this.

I got this by trying to understand what is going on in the obfuscated Google Maps javascript. The testcase seems to have the same properties as the problem in Google Maps and the testcase does work in IE (with changing addEventListener -> attachEvent and textContent -> innerHTML for IE).
Keywords: testcase-wanted
Ah, area handling may need some tweaking.
<area> doesn't have a frame, but is represented in/by nsImageFrame.
Maybe PresShell's retargeting code should check if capturingContent
is an area and if it is, find the relevant imageframe and target to that.
(nsImageFrame::GetContentForEvent might need some changes.)
Though, I'm not sure how to handle the case where the same image map is used
with many images. Have to test how IE handles that case.
<area> doesn't seem to be the problem, it happens for a plain linked image. It seems that we start a drag, and this prevents anymore mouse events from getting to the element that thinks it has captured the mouse.
IE doesn't allow drags when a setCapture is active, and we do, that is the problem. So do we want to match the IE behaviour?
Hmmm, the <area> must be part of it too, as disabling the drag fixes the problem for a plain image, but not an area.
Assignee: nobody → tnikkel
Attachment #420271 - Flags: review?(Olli.Pettay)
This code I'm not very familiar with, so I don't know if this is a safe thing to do.
Attachment #420273 - Flags: review?(Olli.Pettay)
Hmmm, the disabling of drag when mouse capture is active patch isn't actually needed to fix the Google Maps issue. But it does represent a difference between Firefox and IE with regards to setCapture.
Comment on attachment 420271 [details] [diff] [review]
special case <area>'s that are capturing the mouse

So area's primary frame is the image frame?
This definitely needs tests.
Comment on attachment 420273 [details] [diff] [review]
don't allow a drag to start when a mouse capture is active

This should work, even with scrollbars.
(See nsXULElement::PreHandleEvent)

Enn should review this too.

And some testcase would be great.
Attachment #420273 - Flags: review?(Olli.Pettay) → review+
(In reply to comment #17)
> So area's primary frame is the image frame?

Yeah, due to bug 135040.
Another set of Google Maps steps that regressed on the same day, but don't seem to be fixed by the pair of patches above:
 (1) while logged in to google, create a map using the "My Maps" - "Create a Map" feature in http://maps.google.com/
 (2) create at least 2 placemarks in that map
 (3) while editing, drag the placemarks up and down to reorder them

The dragging of placemarks while editing a map broke in the same regression range, but isn't fixed by attachment 420271 [details] [diff] [review] plus attachment 420273 [details] [diff] [review].

Should this be a separate bug?
(In reply to comment #20)
> Should this be a separate bug?

Yes, I think so, could you file it?.

The dragging patch should maybe be in its own bug too.
Comment on attachment 420271 [details] [diff] [review]
special case <area>'s that are capturing the mouse

So if you could add some tests for this, r=me.
Attachment #420271 - Flags: review?(Olli.Pettay) → review+
>+    // If something is capturing the mouse don't start a drag.
>+    if (nsIPresShell::GetCapturingContent()) {
>+      StopTrackingDragGesture();
>+      return;
>+    }

Currently, we do the reverse where starting a drag cancels the capture, but this addition looks ok if it's what IE does, which seems better anyway.
Added a test, and I only got bitten by image maps sucking two more times in doing so.
Attachment #420271 - Attachment is obsolete: true
Attachment #422664 - Flags: review?(Olli.Pettay)
Added a test.
Attachment #420273 - Attachment is obsolete: true
Attachment #422665 - Flags: review?(Olli.Pettay)
Attachment #422665 - Flags: review?(enndeakin)
Comment on attachment 422664 [details] [diff] [review]
special case <area>'s that are capturing the mouse v2

Forgot to mention, multiple images using the same image map actually does work with capturing after all, because the event frame gets set to whichever image is the primary frame of the area element (even if the event is to the other image), and it redirects to the area when it is capturing.
Attachment #422665 - Flags: review?(enndeakin) → review+
Attachment #422665 - Flags: review?(Olli.Pettay) → review+
Attachment #422664 - Flags: review?(Olli.Pettay) → review+
Keywords: checkin-needed
I hope to land this in the next few days.
The disabling of drag when a mouse capture is active patch causes content/events/test/test_dragstart.html to fail. This is because in nsFrame::HandlePress we capture on the nearest scroll frame when making a selection at
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp#1900 and we then refuse to start a drag because this capture is active.

We could keep track of if the current mouse capture came from native code or js and then only deny drags during js setCapture's.
Keywords: checkin-needed
Used the idea in comment 29 to pass test_dragstart.html.
Attachment #422665 - Attachment is obsolete: true
Attachment #424368 - Flags: review?(enndeakin)
Comment on attachment 424368 [details] [diff] [review]
don't allow a drag to start when a mouse capture is active v3

>-// 06AA90C2-5234-4F1C-81D7-773B5E4CBB8B
>+// 5086B1D0-4969-41E9-8630-A93881888336
> #define NS_IPRESSHELL_IID     \
>-{ 0x06aa90c2, 0x5234, 0x4f1c, \
>- { 0x81, 0xd7, 0x77, 0x3b, 0x5e, 0x4c, 0xbb, 0x8b } }
>+{ 0x5086b1d0, 0x4969, 0x41e9, \
>+ { 0x86, 0x30, 0xa9, 0x38, 0x81, 0x88, 0x83, 0x36 } }

You haven't changed any interface methods so you shouldn't need to update the iid.


>+   *
>+   * CAPTURE_NOTNATIVE should be set when called from non-native code, i.e.
>+   * element.setCapture().

I don't really like the 'native' name here as it could be confusing since element.setCapture can be called by native code as well. It would be better to be more specific as to what the flag does with a name like 'CAPTURE_PREVENTDRAG'. Or perhaps the reverse for the other callers with 'CAPTURE_ALLOWDRAG'. Do all of the other 'native' callers want to allow dragging as well?


>+<hbox>
>+  <img id="image" xmlns="http://www.w3.org/1999/xhtml"
>+       onmousedown="this.setCapture();" onmouseup="this.releaseCapture();"
>+       ondragstart="ok(false, 'should not get a drag when a setCapture is active');"
>+       src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAIAAAAC64paAAAAG0lEQVR42mP8z0A%2BYKJA76jmUc2jmkc1U0EzACKcASfOgGoMAAAAAElFTkSuQmCC"/>
>+</hbox>
>+
>+

Remove the extra blank line.
Attachment #424368 - Flags: review?(enndeakin) → review+
(In reply to comment #31)
> You haven't changed any interface methods so you shouldn't need to update the
> iid.

Ok.

> I don't really like the 'native' name here as it could be confusing since
> element.setCapture can be called by native code as well. It would be better to
> be more specific as to what the flag does with a name like
> 'CAPTURE_PREVENTDRAG'. Or perhaps the reverse for the other callers with
> 'CAPTURE_ALLOWDRAG'. Do all of the other 'native' callers want to allow
> dragging as well?

That is a good idea. I went with CAPTURE_PREVENTDRAG. I had a look at all the other callsites and I don't know if any of them want to prevent or allow drag, so I'm just going to leave them alone in absence of any specific reason to change them.

Also, I called the nsIPresShell function IsMouseCapturePreventingDrag and made it check both for the prevent drag and having a current capturing content.

> Remove the extra blank line.

Done.

Thanks for the review.
blocking2.0: ? → beta1
http://hg.mozilla.org/mozilla-central/rev/9fa71ec53b78
http://hg.mozilla.org/mozilla-central/rev/9e8ac2f9d40d
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
It seems that this bug has reappeared in Firefox, beginning with version 8.0.

Usually (IE, Firefox 7 and below) if you go along the blue line of a route planned by Google Maps, a small circle appears which can be dragged and dropped to other roads so that the route planner changes the route. This is impossible with Fx 8, the circle doesn't appear and when trying to drag&drop I only move the map. The same applies to the markers which mark the start and destination of the route, they are not visible and cannot be moved. Disabling Adblock and Firegestures didn't solve the problem. When the page is loading for a moment you can see the symbol for an image which is not yet loaded both where the markers should be and if you go along the line with the mouse. As soon as the page is completely loaded you don't see this anymore. 

Important: It seems to have something to to with Proxy authentication. In the Web Console, the following error messages appear when loading Google Maps:

[20:55:53.119] GET https://maps.google.com/images/experiments/nav_logo78.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[20:55:54.991] GET https://maps.google.com/maps/gen_204?imp=ael&jsv=386c [HTTP/1.1 407 Proxy Authentication Required 0ms]
[20:55:56.173] GET https://maps.google.com/maps/gen_204?imp=mapsgl:promo:show&ei=R10DT6zCDM7J_AblwOSlCA [HTTP/1.1 407 Proxy Authentication Required 0ms]

After a route was entered and computed, the following messages appear in the Web Console when the page is rendered:

[15:26:26.816] GET https://maps.gstatic.com/mapfiles/markers2/red_markers_A_J2.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:26.833] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:27.213] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:27.264] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:44.612] GET https://maps.gstatic.com/mapfiles/transparent.png [HTTP/1.1 407 Proxy Authentication Required 15ms]
[15:26:44.629] GET https://maps.google.de/maps/trends?output=thumbnails&panoramio=11797888&client=maps [HTTP/1.1 407 Proxy Authentication Required 15ms]
[15:26:45.487] GET https://maps.gstatic.com/mapfiles/markers2/red_markers_A_J2.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:45.538] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 15ms]
[15:26:45.599] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:45.660] GET https://maps.gstatic.com/mapfiles/markers2/markers_A_J2_B254FD.png [HTTP/1.1 200 OK 0ms]
[15:26:45.732] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:45.768] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:45.884] GET https://maps.gstatic.com/mapfiles/markers2/red_markers_A_J2.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:45.969] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:46.003] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:47.312] GET https://maps.gstatic.com/mapfiles/markers2/markers_A_J2_B254FD.png [HTTP/1.1 407 Proxy Authentication Required 16ms]
[15:26:47.364] GET https://maps.gstatic.com/mapfiles/markers2/markers_A_J2_B254FD.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:53.888] GET https://maps.gstatic.com/mapfiles/markers2/red_markers_A_J2.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:53.930] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:53.969] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:54.063] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:54.115] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:54.185] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:54.387] GET https://maps.gstatic.com/mapfiles/markers2/red_markers_A_J2.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:54.432] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:26:54.840] GET https://maps.gstatic.com/mapfiles/markers2/markers_A_J2_ABE457.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:05.919] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 16ms]
[15:27:05.975] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:06.024] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:06.084] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:06.145] GET https://maps.gstatic.com/mapfiles/transparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:06.227] GET https://maps.gstatic.com/mapfiles/markers2/red_markers_A_J2.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:06.262] GET https://maps.gstatic.com/mapfiles/markers2/shadow50.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:06.311] GET https://maps.gstatic.com/mapfiles/markers2/markerTransparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:06.971] GET https://maps.gstatic.com/mapfiles/markers2/marker_greenA.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:07.028] GET https://maps.gstatic.com/mapfiles/markers2/marker_greenB.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:07.089] GET https://maps.gstatic.com/mapfiles/markers2/dd-via.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:07.488] GET https://maps.gstatic.com/mapfiles/markers2/dd-via.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:07.541] GET https://maps.gstatic.com/mapfiles/markers2/drag_cross_67_16.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
[15:27:07.597] GET https://maps.gstatic.com/mapfiles/markers2/dd-via-transparent.png [HTTP/1.1 407 Proxy Authentication Required 0ms]
Thank you for the bug report, Gerd! Since the original issue in this bug is resolved, could you open a new bug to track your issue? It sounds like something we should be able to isolate fairly quickly if it definitely did not used to occur in FF7 but does in FF8. Please add me to the CC field of the new bug as well, so I can follow up on it.
What is the new bug #, so I can track it.  It is very annoying to have to switch to IE to use the drag functionality.
Bug 715124 was the followup that was filed. Thanks for reminding me to add it here.
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.