441590 - Always retain a border around the edges of the screen to be used for panning (scroll google maps)

Status: RESOLVED DUPLICATE of bug 544614

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Doug Turner (:dougt)]

I can not use the maps widget on maps.google.com

1) load maps.google.com
2) try to zoom in, zoom out, or pan the map.

Comment 1

[Rubén Martín [:Nukeador] ❌ [away till Aug 31st]]

The main problem with Google Maps and Fennec is that you can't drag the map around because Fennec thinks you want to show the toolbars by dragging around.

Comment 2

[Madhava Enros [:madhava]]

I think what we want is that panning on a pannable area (like an iframe) should pan that area, rather than the whole page.

Comment 3

[Oleg Romashin (:romaxa)]

This is not iframe, this is scrollable DIV...
And even if we will scroll that DIV (ignoring overflow hidden) it will not help, because in that case we can see only small part of prerendered  maps area...

Only one way will allow us to use such creatures as google/yahoo maps is creation on some special mode which will allow user to interact with content without panning ( send mouse events directly to content).

Comment 4

[YUKI "Piro" Hiroshi]

An idea, how about "pinned" mode? For example...

 * When we click "pin" button in the toolbar, an icon of pin is shown on
   a edge of the pannable area (BrowserCanvas).
 * While the pin appears, we cannot pan the area and Fennec sends any
   mouse events to the content directly from dragging or tapping.
   (So, now we can use Google Maps!)
 * We can remove the pin by tapping on it. After the pin disappeared,
   the area (canvas) becomes pannable again.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #4)
> An idea, how about "pinned" mode? For example...

Yes, we have been talking about the "pinned" idea for a while now. There is disagreement over using an explicit UI button to switch modes. The ideal approach would be seemless to the user.

I would definitely want to make sure an extension could add the "pinned" mode. We need to make sure the code is capable of turning panning off and on.

Comment 6

[YUKI "Piro" Hiroshi]

As an extension author, I hope the "pinned" mode is added as an feature of Fennec itself. In other words, I think it is danger that such a basic feature is introduced by addons.

For example, Tab Mix Plus (and old Tabbrowser Extensions) for Firefox. Because it changes too many codes of Firefox, we addon authors are troubled by compatibility problem about TMP. I'm afraid that two things: the "pin" extension possibly becomes an TMP for Fennec (because I think that adding "pin" mode require many changes), and compatibility issues possibly deaden our enthusiasm...

Comment 8

[Ben Combee (:bcombee)]

Talking during the UI sprint -- if we implemented some sort of floating toolbar mechanism to support zoom on single-touch screens, we could also add the "drag/pan" toggle button to that.

Comment 9

[Ben Combee (:bcombee)]

Talked with madvaha about this a bit today -- the iPhone 3.0 browser is able to scroll Google Maps.  It seems to be sending the events first to the content page and them using them for panning if they aren't absorbed by the JS handlers.

We should look at doing a proof-of-concept of this mechanism.  I'm concerned about two things:

1) it would slow down panning because we'd have to wait for content to handle the events before we can use them in our chrome code

2) the user could get into a situation where they couldn't interact with our UI anymore because the content was absorbing everything.  It would be easy to make a test page that made you quit the browser to continue using the device.

Comment 10

[Oleg Romashin (:romaxa)]

(In reply to comment #9)
> Talked with madvaha about this a bit today -- the iPhone 3.0 browser is able to
> scroll Google Maps.  It seems to be sending the events first to the content
> page and them using them for panning if they aren't absorbed by the JS
> handlers.

iPhone 3.0 google maps scrolling - is just fake, and it seems hardcoded somewhere.
Easy to check - Open maps.yahoo.com, maps.yandex.ru e.t.c.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

vote for not blocking1.0

Comment 12

[René-Luc]

I think for geolocation is blocking if the user can't drag the map.

What about a css property to decalre that a <div/> could receive mouse event (mousedown, mousemove, mouseup, dblclick) ?

Comment 13

[Ben Combee (:bcombee)]

René-Luc, even if you had the CSS attribute, you need a mechanism to tell the browser that a pan shouldn't go to the div... otherwise you can get into a mode where you can't scroll away from the content back to the browser controls.

Comment 14

[René-Luc]

During the European MozCamp in Prague I'm discussing about the mechanism fennec draw web site and dispatch event. And it's right we need to define a mechanism to activate and deactivate web map panning.

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

FWIW, I think this needs to be a 1.0 blocker.  Not being able to demonstrate working google maps while we're talking about being a "full HTML5 browser" really doesn't make sense.

As a simple stopgap I would suggest a button somewhere in the UI that:

1) disables panning and passes events to the page (perhaps letting you hold down a button to pan?)

2) forces the URLbar to always be visible, where there would be another button that would get you out of this mode

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

this is in litmus on the web usage part of the BFT.  There is a google maps testcase:
https://litmus.mozilla.org/show_test.cgi?id=7576

Comment 17

[Aakash Desai [:aakashd]]

The testcase was using the direction pad on the top left side of the map to move around the map, but I'm under the impression that this bug is supposed to take care of panning within the map. 

I've updated the testcase, so moving this to +

Comment 18

[Fabrice Desré]

Attachment: wip

This patch is a wip with the following features :

It tries to guess if a mousedown target is a pannable element using a basic heuristic based on the element's id and the event listeners. When such an element is found, we forward mousedown, mousemove and mouseup directly to the browser until mouseup.

This works with http://maps.yahoo.com/ and http://maps.yandex.ru/

On http://maps.google.com/ the move starts with south jump ??? And we don't get the final mouseup event, hence the ugly hack in InputHandler::handleEvent()

It doesn't work at all with http://www.openstreetmap.com/

Comment 19

[René-Luc]

I'm testing the fabrice'patch on Linux.

All it's ok for http://maps.google.com, http://maps.yahoo.com and http://www.bing.com/maps/

For OpenStreetMap, the problem comes from OpenLayers. OpenLayers makes it easy to put a dynamic map in any web page. I made some test with OpenLayers and the problem with OpenLayers map is that it receives 'mouseout' event.

Comment 20

[:Felipe Gomes (needinfo for replies!)]

FWIW, GMaps works on mobile Safari by using their Touch API.  The panning can be disabled if the page calls preventDefault() on the touchmove event.

Comment 21

[Antonio Gomes (tonikitoo)]

(In reply to comment #20)
> FWIW, GMaps works on mobile Safari by using their Touch API.  The panning can
> be disabled if the page calls preventDefault() on the touchmove event.

webkit based browsers have a different approach: a page-level panning  will only be occur if event was not internally handled by webcore.

so it implicitly does exactly what felipe described.

Comment 22

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: wip-1

The patch works on maps.google.com && maps.bing.com.
It did not use the mouseup hack and has only a a few pixel south/north jump (depending on the zoom)

This also use safety barriers on every corner (64px each) to always allow panning.

Comment 23

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

This patch dispatch mousemove to content that request for such an event for dragging operations.

This did not support every webapps but it works on some popular (maps.google.com && maps.bing.com).

Comment 24

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch for device

This one works on device, not sure why the event on the y-axis are different between the device and the desktop. If perfs for this patch are ok, i think we can use some ifdef for now.

Comment 25

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.3

This one should work on both

Comment 26

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 436469 [details] [diff] [review]
Patch v0.3

>diff -r 50a98e0c32b8 chrome/content/InputHandler.js
>+/**
>+ * PageInteractionModule
>+ */

A little more descriptive comment would be good here. What does this module do?

>+function PageInteractionModule(owner, browserViewContainer) {

>+  this.allowUntrustedEvent = true;

Add a comment above this line to explain why we need to allow untrusted events

>+  _targetIsSafeArea: function _targetIsSafeArea(aEvent) {
>+    const kSafety = 64;
>+    return (aEvent.clientX < kSafety || (window.innerWidth - aEvent.clientX) < kSafety ||
>+            aEvent.clientY < kSafety || (window.innerHeight - aEvent.clientY) < kSafety);
>+  },

Add a comment to describe what the "safety" area does

>+  _isListeningContent: function isListeningContent(clientX, clientY) {

>+    let els = Cc["@mozilla.org/eventlistenerservice;1"].getService(Ci.nsIEventListenerService);

Add a lazy property?

>+    let chains = els.getEventTargetChainFor(element, {});
>+    for (let i = 0; i < chains.length; i++) {
>+      let el = chains[i];
>+      if (el instanceof HTMLHtmlElement)
>+        continue;
>+
>+      if (el instanceof HTMLElement) {
>+        let listeners = els.getListenerInfoFor(el, {});
>+        for (let i = 0; i < listeners.length; i++) {
>+          let listener = listeners[i].type;
>+          if (listener == "mousemove") {
>+            return el;
>+          }
>+        }
>+      }
>+    }

I wonder how slow this is in general? Are we doing all that we can to avoid even getting this far, if possible? (just thinking out loud)

>+  handleEvent: function handleEvent(evInfo) {
>+    let evt = evInfo.event;
>+    if (evt.button !== 0)
>+      return;
>+
>+    let clicker = Browser._browserView._container.customClicker;
>+    switch (evt.type) {
>+      case "mousedown":
>+          if (!(this._targetIsContent(evt) && this._isListeningContent(evt.clientX, evt.clientY)) || this._targetIsSafeArea(evt))
>+            return;
>+
>+            // XXX ugly hack, we need it to fire the onresize handler on some app
>+            let contentWindow = Browser.selectedBrowser.contentWindow;
>+            if (!contentWindow.___resizehasbeendone___) {
>+              contentWindow.resizeBy(0,1);
>+              contentWindow.resizeBy(0,-1);
>+              contentWindow.___resizehasbeendone___ = true;
>+            }

Use "__PI_resized" instead of "___resizehasbeendone___" - SessionsStore adds properties like this too and uses this convention.

I'm in the process of testing this on desktop and device. So far, it's OK.

Comment 27

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.4

This patch address comments and avoid open the context menu during a pan. It is also noticeably faster than the previous one.

This is still a first way into interaction, it is still hard to use google maps on device (no dblclick, no context menu, ...).

Also, this patch won't work on some desktops, where we gonna see "south jump" for now. I don't think this is very important for now and I can correct that later.

Comment 28

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 436616 [details] [diff] [review]
Patch v0.4

I've forgot to ask a review for this patch.

Comment 30

[Mark Finkle (:mfinkle) (use needinfo?)]

This bug won't make it for Fennec 1.1

Comment 31

[Juha Seppänen]

Seeing this now on Fennec nightlies like 20100718.

Comment 32

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 436616 [details] [diff] [review]
Patch v0.4

The patch is hardly bitrotted and I'm not working actively on that.

Comment 33

[Doug Turner (:dougt)]

madhava, do we have a plan here?

Comment 34

[Doug Turner (:dougt)]

no blocking.

Comment 35

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

hater.

Comment 36

[Aakash Desai [:aakashd]]

haters gonna hate

Comment 37

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Aakash, imma let you finish, but you gotta blame the game.

Comment 38

[Doug Turner (:dougt)]

the game has multiple levels, we'll get this solved in the next one.

Comment 39

[Aakash Desai [:aakashd]]

yo dougt, you gotta stop this rap because you got no rhymes.

Comment 40

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

the princess will always just be in another castle

Comment 41

[Madhava Enros [:madhava]]

(In reply to comment #33)
> madhava, do we have a plan here?

for reference, I think it was the same plan as last time, which is to pass panning through to the web app unless they start in a band around the edge of the viewport.

Comment 42

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #41)
> (In reply to comment #33)
> > madhava, do we have a plan here?
> 
> for reference, I think it was the same plan as last time, which is to pass
> panning through to the web app unless they start in a band around the edge of
> the viewport.

which is nicely described in bug 531974

Comment 44

[Mark Finkle (:mfinkle) (use needinfo?)]

I like beltzner's title better

Comment 47

[Robert Kaiser]

We need some way for web apps to be driven with mousedown/move/up events (didn't we just make a web gaming contest and shouldn't those work with Fennec as much as possible?) as well as (which is how I stumbled over this) ways for in-content UI to consume those panning events (it's surely nicer to have extensions show in-content "tabs" with their UI instead of opening new windows, which doesn't make for good Fennec UX). On the other hand, we need to ensure there's still a way to make our UI appear.
If we want to have Firefox 4 Mobile gain a significant amount of users, I'm pretty sure this should work - but then I know we have only so many people who can work on such things. I just hope we can find a solution here.

Comment 48

[Cristian Nicolae (:xti)]

This issue is still reproducing on:
Build id : Mozilla/5.0 (Maemo;Linux armv7l;rv:2.0b13pre)Gecko/201100316
Firefox/4.0b13pre Fennec /4.0b6pre
Device: Sony Ericsson Xperia X10
OS: Android 2.1 update 1

Comment 50

[Mark Finkle (:mfinkle) (use needinfo?)]

Work now in bug 544614