Closed Bug 827270 Opened 12 years ago Closed 11 years ago

Cannot scroll transit directions in the maps app

Categories

(Web Compatibility :: Site Reports, defect)

Product:
Web Compatibility
Component:
Site Reports
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-basecamp:-)

Status:
RESOLVED FIXED
Project Flags:
blocking-basecamp -

People

(Reporter: jsmith, Unassigned)

References

Details

(Keywords: regression) 

Breakout on Andrew's test case with the maps app.

Steps:

1. Go to the maps app
2. Get transit directions from point X to Y 
3. Go to the list view after you get results

Expected:

The results should be scrollable.

Actual:

The results are not scrollable. This looks like a different bug than bug 824699 after confirming that this bug did not happen on 12/20, but the fullscreen bug in bug 824699 occurred. If I end up finding out I'm not right, feel free to dupe.
blocking-basecamp: --- → ?
So far we know:

12/20/2012 - Working
1/6/2013 - Busted
Keywords: regression, regressionwindow-wanted
Jet, can you help find someone to work on this?
Assignee: nobody → bugs
blocking-basecamp: ? → +
Keywords: qawanted
QA Contact: jsmith
For more information, the list view is scrollable if you press and drag at the above blue area.
The problem appears to be caused by:

commit 2825edd96e166d08598fd9e2738c81c7a94ac0c3
Author: Chris Peterson ext:(%2C%20Shih-Chiang%20Chien%20%3Cschien%40mozilla.com%3E%20and%20Chris%20Jones%20%3Cjones.chris.g%40gmail.com%3E) <cpeterson@mozilla.com>
Date:   Fri Jan 4 17:29:31 2013 -0800

    Gecko work for bug 823619: Dispatch spec-compliant mouse events when touch events are available. r=cjones,jlebar,schien,vingtetun a=blocking-basecamp
    
    This is a rollowup of the following patches
    Bug 823619, part 1: Make TabChild dispatch spec-compliant compat mouse events. r=mwu
    Bug 823619, part 2: Use touch event for scrolling if available. r=cjones,schien,vingtetun a=blocking-basecamp
Blocks: 823619
Anthony: you can probably fix this just by adding shared/js/mouse_event_shim.js to the app.  We shimmed apps in the apps/ directory, but didn't look for apps that might be affected in other directories.

See my post to dev-gaia (subject: "Please Read: breaking change to mouse events"). And if you have other questions, feel free to ask me.
The maps app is at http://maps.nokia.com. How do we get *that* fixed? Andrew, do you know?
Oh. I haven't worked with that kind of app before, so I may not know what I'm talking about.

Were they writing it just for our platform? We had non-standard mobile events before. Now we're standard, so if they can run in mobile safari, for example, they ought to run on our platform. Is there browser sniffing going on do you think?

Can we view source their code or is it obfuscated?
The source is minified, at least.

Andrew, can you connect us to Nokia here?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> The source is minified, at least.
> 
> Andrew, can you connect us to Nokia here?

Putting needsinfo on Donovan - he's been talking to Nokia.

Also removing qawanted and regressionwindow-wanted - comment 4 gives the root cause of the regression.
Flags: needinfo?(dpreston)
Keywords: qawanted, regressionwindow-wanted
Ok, so it looks like we just need to get them to include shared/js/mouse_event_shim.js in Maps? I'll talk to them about it.

Also, I noticed when I built and flashed from source today that the settings app did not scroll, is it perhaps related to this
Flags: needinfo?(dpreston)
It seems like this might keep coming up for new apps that haven't yet included mouse_event_shim.js. Is there a more transparent solution in the works?
Apps should now be written to use touch events instead of mouse events, following the mobile web conventions established by the iPhone and followed by everyone including Firefox for Android.

The mouse event shim is only needed for apps that use mouse events instead.

Donovan: you might refer your Nokia contacts to my dev-gaia post.
Thanks, David! I pointed them at the bug, so I'll link to it here:

https://groups.google.com/d/msg/mozilla.dev.gaia/as_XclduM78/0kSNac0RC5MJ
I've just read the steps to reproduce more carefully, and I'm not sure that this has anything to do with the mouse event shim...  Since it is a maps app, I was assuming that the problem was panning a map.  But now that I've actually read and see that the issue is scrolling a list, I'm no longer as sure that is it.

It looks to me as if that list of directions could just be put in a div with overflow-y:scroll and it would just work. If that is broken, this is bad.

But if the maps app is doing its own scrolling with mouse events, then Nokia could port to use touch events, or could try the mouse_event_shim.
Component: DOM → Gaia
Product: Core → Boot2Gecko
Version: Trunk → unspecified
David, I think you're a better owner for this, if you're not overloaded.
Assignee: ajones → dflanagan
Hi all, I've tried it out with the latest OTA update (build 20130104070203) and I cannot replicate the issue. 

The list scrolling in directions should be the same as the list scrolling for normal search results, where we are simply using document scrolling. It would be weird if that needs any work-around in a browser.
(In reply to Andy from comment #16)
> Hi all, I've tried it out with the latest OTA update (build 20130104070203)
> and I cannot replicate the issue. 
> 
> The list scrolling in directions should be the same as the list scrolling
> for normal search results, where we are simply using document scrolling. It
> would be weird if that needs any work-around in a browser.

Adding qawanted to verify this. This bug should be WFM if verified.
Keywords: qawanted
With latest OTA update (20130109071737) on Otoro I cannot scroll search result nor directions on the Map app :'(
Keywords: qawanted
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> David, I think you're a better owner for this, if you're not overloaded.

I'm not overloaded, but I don't know if I can do anything about it other than evangelize Nokia about it...

Donovan: would you email me your contact info for the developer at Nokia?
David: I already did and Andy replied in comment 16.

I'll take this since I'll make sure a fix lands
Assignee: dflanagan → dpreston
Sounds like this is evangelism now. No need to stop ship for Jan 15th on this. But we definitely need evangelize this, though.
blocking-basecamp: + → ---
(In reply to Andy from comment #16)
> Hi all, I've tried it out with the latest OTA update (build 20130104070203)
> and I cannot replicate the issue. 
> 
> The list scrolling in directions should be the same as the list scrolling
> for normal search results, where we are simply using document scrolling. It
> would be weird if that needs any work-around in a browser.

Hi Andy, I know it is weird but can you try adding mouse_event_shim and we will see if that helps.
(In reply to Donovan Preston from comment #20)
> David: I already did and Andy replied in comment 16.

Sorry, I misread David's comment, I thought he said "would you email your contact" not "would you email me your contact info"
My reading of comment 16 is, "we're using a normal scrollable frame".  I.e., "go fix your broken rendering engine" ;).
(In reply to Andy from comment #16)
> Hi all, I've tried it out with the latest OTA update (build 20130104070203)
> and I cannot replicate the issue. 
> 
> The list scrolling in directions should be the same as the list scrolling
> for normal search results, where we are simply using document scrolling. It
> would be weird if that needs any work-around in a browser.

Hi Andy!

If your app relies on mousemove, mouseover, mouseout, mouseenter, or mouseleave events, then you should try the shim.  But it sounds like the scrolling is just normal document scrolling, so that is unlikely to be the issue here. (Still you should try the shim if nothing else works for you...)

If your app ever calls preventDefault on a mouse or touch event, that might well prevent scrolling from happening.

Of course you need to be able to duplicate the bug before you can fix it. What device are you testing on?  Tim says he can duplicate on Otoro. I've seen it on my Unagi.  Are you using something else?

If you've got a non-minified version of the app that you can share, we can try to help on this end.

And the fact that cjones has just commented on the bug means that you're in good hands if the bug is actually on our end.  But do check for preventDefault in your app!
Ok, we will add the shim to a branch and send it to Donovan and you to test it, because our device doesn't update to this build. (I am not sure whether our device is Otoro or Unagi - Donovan probably knows which one we have.)
(In reply to Andy from comment #26)
> Ok, we will add the shim to a branch and send it to Donovan and you to test
> it, because our device doesn't update to this build. (I am not sure whether
> our device is Otoro or Unagi - Donovan probably knows which one we have.)

It's an unagi. Strange it is not updating to the latest build. Maybe it is on a "stable" channel? I don't know much about the update mechanism.
If I visit maps.nokia.com in the FirefoxOS browser, scrolling works fine. But not when I open the app from the homescreen.

Can we still bookmark pages from the browser? I'd try that, but can't figure out how to create a bookmark.
(In reply to David Flanagan [:djf] from comment #28)
> If I visit maps.nokia.com in the FirefoxOS browser, scrolling works fine.
> But not when I open the app from the homescreen.

Hmm. Now that changes things - now this sounds like it's a gecko bug. Evangelism wouldn't solve a problem where it works in the browser, not in the app for this example.

> 
> Can we still bookmark pages from the browser? I'd try that, but can't figure
> out how to create a bookmark.

Yeah. Click the star button, select add to bookmark. Then launch that site from the bookmark on the homescreen.
blocking-basecamp: --- → ?
Component: Gaia → General
roc,

Dale Harvey said he was going to work on this in the morning (Berlin time).  That shouldn't prevent you from fixing it while he sleeps though :-)
It works when I add the site to the homescreen as a bookmark. So it is only broken when running as an app.

As an app it used to have a different mouse event model than it would have in the browser. So maybe there is code in there to work around that that is now causing this problem.

Andy, were you serious about sending us a modified branch of the code to test?
Dale, do you have a plan here?

Shih-Chiang, you have another blocker ... can you think of someone else to help here with potential fallout from bug 823619?
blocking-basecamp: ? → +
Flags: needinfo?(dale)
The Nokia team are apparently coming into berlin this morning I am hoping I can get at their source code to see if there is anything that would get in the way of our event handling, but I suspect that its a plain gecko bug. Looking into BrowserElementScrolling.js now
Flags: needinfo?(dale)
(In reply to Dale Harvey (:daleharvey) from comment #33)
> The Nokia team are apparently coming into berlin this morning I am hoping I
> can get at their source code to see if there is anything that would get in
> the way of our event handling

That's awesome!
(In reply to Andrew Overholt [:overholt] from comment #32)
> Dale, do you have a plan here?
> 
> Shih-Chiang, you have another blocker ... can you think of someone else to
> help here with potential fallout from bug 823619?
All the people I know are handling other bb+ blockers right now. I think Dale is taking the right direction for investigating this bug.
(In reply to Shih-Chiang Chien [:schien] from comment #35)
> (In reply to Andrew Overholt [:overholt] from comment #32)
> > Dale, do you have a plan here?
> > 
> > Shih-Chiang, you have another blocker ... can you think of someone else to
> > help here with potential fallout from bug 823619?
> All the people I know are handling other bb+ blockers right now. I think
> Dale is taking the right direction for investigating this bug.

Agreed.  jdm (Josh Matthews) has offered to help with this and the other scrolling issues.
A small update: we do not have any code that handles events differently in app mode than in the browser. (so no work-arounds for the previous model). 

We now have updated phones and will try out the shim and report back here.
Another update: we added the script tag for the shim in one of our branches. But the way it was instructed in the Google Group cannot work:
<script src="shared/js/mouse_event_shim.js"></script>  is pointing to our server, where we do not have that shim. What is the correct url to this javascript file?
I replied to the email but for completeness will reply here too, the shim is at 

https://raw.github.com/mozilla-b2g/gaia/master/shared/js/mouse_event_shim.js
We are missing a touchstart (and a touchend) event in BrowserElementScrolling.js

Inside the browser and bookmarked apps the asyncpanandzoomcontroller handles the touchstart before content sees it, but inside apps we wait until the event is propogated up from the content which is why bookmarks + browser scroll fine.

There must be a stopPropagation called within a touchstart event somewhere in the maps code which needs to be removed.
We managed to add the shim to the branch (which we sent to Dale and Donovan), but the issue is not resolved by the shim.
Yeh the shim wouldnt help for your situation, you are already using touchevents, we dont scroll if touchstart has been stopPropogated which I am fairly certain is what is happening.

With the full app source code I could debug this further, but without it there isnt anything more I can do than to say to look at the touchstart handling code.

Renomming as this looks like an evangelism bug
blocking-basecamp: + → ?
Dale, have you had a chance to look at the source code that I sent you and iscroll 4.2.2 ?
Yes sorry in ontap.js you have a call to stopPropogation called ontouchstart, if that is active on the list then it will almost certainly cause this bug
Yes, that's right. I also nailed down this problem to ontap.js. Removing stopPropogation solved the problem. Thanks guys.
Yes sorry in ontap.js you have a call to stopPropogation called ontouchstart, if that is active on the list then it will almost certainly cause this bug
hah sorry I had this comment in a tab and forgot to submit, glad you found it, cheers

Can you update this bug once you have deployed the fix and we can mark it resolved
blocking-basecamp: ? → -
Component: General → Mobile
Product: Boot2Gecko → Tech Evangelism
This might be a bigger issue though. Let me explain what we do in ontap.js:
on iOS & Android touch devices, we found out that listening to click-events results lags up to 300ms in ui response. This is a common problem on touch platforms. Even on FirefoxOS there is an extra delay of 100ms. A solution for this is to create your own custom 'tap' events that basically deduct 'ontap' by listening to ontouchstart, ontouchend etc. and calculate whether we can interpret it as a 'tap'. Other frameworks like jquery-mobile also use this method.

Now, sometimes we have an element in HERE Maps that needs to react on a tap, but that element also contains elements that need to react differently to a tap. (e.g. list items that contain buttons and different tap areas) In cases like these, we need to use stopPropagation on some events from the contained buttons.

So, although I fully agree that web devs on touch devices should use touch events:
- it cannot be that you prohibit those devs from using stopPropagation on any touch event.
- it cannot be that developers need to consider a different implementation in the browser/bookmarked and app mode 

We are now creating a workaround, especially for Firefox OS: we will use 'click' events instead of touch events to detect taps on list items. But this does mean that we will get a ui response delay of up to 100ms on Unagi. 

Let us know if there is a better way to fix this.
Vivien/Shih-Chiang: could we add a capturing listener for touchstart to BrowserElementScrolling, which would possibly enable panning, and then use the existing bubbling listener to detect preventDefault()?

(While this worked before bug 823619, it sort of worked by accident :/.)
Flags: needinfo?(21)
Hi guys, just wanted to let you know that the work-around for items in scrollable lists (not reacting to touch-events anymore, but to clicks) was deployed last friday. Go to  m.here.com/install.html  to try it out.

Still anxious to know whether this will be solved on the platform side too.
Hi Andy,

I will test the fix ASAP.

I would like to understand better whether the suggestion in comment 49 will help, too.
Good morning.

I have tested the updated m.here.com version and while the directions do scroll, there is a different strange issue where more and more of the screen gets covered by a white rectangle. As you scroll down, the rectangle starts at the bottom and slowly grows until it covers the whole screen.
Donovan, is this in app mode? We have only seen this in the browser/bookmarked mode. (We will first focus on the map mode issues, and later cover the browser ones)
Yes. It was installed as an app.
> As you scroll down, the rectangle starts
> at the bottom and slowly grows until it covers the whole screen.

We cannot replicate in Maps team (nightly build 20130115 - what we did see in the browser was the disappearance of the topbar - this will be fixed asap but not a showstopper for the installed app). 

Also, Donovan noticed yesterday that on his device this behaviour is not seen in the browser.
Comment #52 could be bug 830256. Might want to try the patch there.
The whitespace bug is now fixed by bug 831973 but the original bug still exists -- cannot scroll the transit directions page.

It seems to me that it would be better to see if comment 49 helps rather than continuing to label this an evangelism bug?
I verified that the hosted version at m.here.com/install.html is fixed, and can now scroll. I would still like it if there was a generic platform fix, but all that is needed to get this wrapped up for now is to get a packaged version with the latest code and check it in to gaia.

I will also triple-check this new packaged version should also have the geolocation permission to close bug 833940.
The fixed packaged version was pushed to Gaia master by Chris Jones on Jan 18.
Sounds like we should close this then.
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(21)
Resolution: --- → FIXED
Assignee: dale → nobody
I don't think so, because the fix hasn't landed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Just tested today's build and the latest maps package and scrolling is working in all places. I'm just waiting for it to be added to the marketplace and gaia updated with the new url and then this can be closed.
Pretty sure it's fixed now.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Product: Tech Evangelism → Web Compatibility
Component: Mobile → Site Reports
You need to log in before you can comment on or make changes to this bug.