Closed Bug 844326 Opened 9 years ago Closed 9 years ago

Cannot scroll the Lanyrd Mobile app

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:tef+, firefox20 wontfix, firefox21 wontfix, firefox22 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

VERIFIED FIXED
B2G C4 (2jan on)
blocking-b2g tef+
Tracking Status
firefox20 --- wontfix
firefox21 --- wontfix
firefox22 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: jsmith, Assigned: schien)

References

Details

(Keywords: regression, Whiteboard: [target 28/2])

Attachments

(1 file, 1 obsolete file)

Build: B2G 18 2/22/2013
Device: Unagi

Steps:

1. Install https://marketplace.firefox.com/app/lanyrd-mobile
2. Launch it
3. Try scrolling it

Expected:

It scrolls.

Actual:

It doesn't scroll. Works fine on FF Android. Definitely think this is a regression.
Broken tier 1 app. Needs to block.
blocking-b2g: --- → tef?
Josh / SC, can one of you guys take a look?
I was working on finding the regression window for this issue. The last time the app could be scrolled was on 2013-01-05-03-09-00, master build. The app does not scroll on any builds after that.
I'm guessing this is a fallout of bug 823619.
Blocks: 823619
Hmm...this is almost like the maps equivalent of bug 827270.

Do we do evangelism outreach here to fix this? Or is this a platform bug?
Answered in IRC:

jsmith	cjones: So is bug 844326 then an evangelism problem or a platform problem. It looks like the maps equivalent of bug 827270.

cjones	jsmith, need to find out what the bug is first, but seems like a platform issue
touchstart events have been |stopPropagation()| by the javascript loaded in this app, which is pretty similar to bug 827270. Here is the the JS stack:

(gdb) p xpc_PrintJSStack(cx, true, true, false)
$1 = 0x42890000 "0 anonymous() [\"https://s3.amazonaws.com/static.lanyrd.net/mobile-web-bundle.95f2235b.gz.js\":2]\n    this = [object Object]\n1 anonymous() [\"https://s3.amazonaws.com/static.lanyrd.net/mobile-web-bundle.95f2235b.gz.js\":2]\n    this = [object Object]\n"
Shih-Chiang, please let me know if you're not the best owner here.
Assignee: nobody → schien
blocking-b2g: tef? → tef+
Whiteboard: [target 28/2]
(In reply to Andrew Overholt [:overholt] from comment #9)
> Shih-Chiang, please let me know if you're not the best owner here.
This bug is most likely to be an evangelism problem from my observation in comment 8. Do we have any contact window to the developers of Lanyrd app?
(In reply to Shih-Chiang Chien [:schien] from comment #10)
> (In reply to Andrew Overholt [:overholt] from comment #9)
> > Shih-Chiang, please let me know if you're not the best owner here.
> This bug is most likely to be an evangelism problem from my observation in
> comment 8. Do we have any contact window to the developers of Lanyrd app?

I thought Chris Jones indicated in IRC that this was definitely a platform problem.
stopPropagation() isn't supposed to cancel default pan/zoom behavior.  That's the role of preventDefault().
BrowserElementPannning will not be able to receive touchstart event if the content javascript invokes |stopPropagation()|.

@cjones, we probably need to handle touchstart at capturing phase like what we discussed in bug 827270 previously. https://bugzilla.mozilla.org/show_bug.cgi?id=827270#c49
Comment on attachment 718370 [details] [diff] [review]
detemine scrolling target at capturing phase

Vivien, can you review this (not sure if cjones will be able to do so today)?
Attachment #718370 - Flags: review?(21)
Comment on attachment 718370 [details] [diff] [review]
detemine scrolling target at capturing phase

What about using a systemEventListener instead http://mxr.mozilla.org/mozilla-central/source/content/events/public/nsIEventListenerService.idl#72 that would ensure we receive the event event if the content has called stopPropagation.
Comment on attachment 718370 [details] [diff] [review]
detemine scrolling target at capturing phase

Please re-ask review when you the previous question is answered.
Attachment #718370 - Flags: review?(21)
Comment on attachment 718370 [details] [diff] [review]
detemine scrolling target at capturing phase

>diff --git a/dom/browser-element/BrowserElementPanning.js b/dom/browser-element/BrowserElementPanning.js

>+  onTouchStart_defaultPrevented: function cp_onTouchStart_defaultPrevented(evt) {

>+    this._resetActive();
>+    this.dragging = false;
>+    this.detectingScrolling = false;
>+    this._activationTimer.cancel();

We should share this code with onTouchEnd().

Looks good, r=me with that.
Attachment #718370 - Flags: review?(jones.chris.g) → review+
Taking the advice in comment 17 from Vivien, we can avoid all the wired mouse/touch event sequences caused by stopPropagation().

https://tbpl.mozilla.org/?tree=Try&rev=5a2bbaf14ae6
Attachment #718370 - Attachment is obsolete: true
Attachment #718820 - Flags: review?(21)
Attachment #718820 - Flags: feedback?(jones.chris.g)
Comment on attachment 718820 [details] [diff] [review]
use addSystemEventListener to receive all touch events without affecting by stopPropagation.

Seems like a better idea to me, but I don't know how system event listeners fit into DOM event semantics.

Are you sure this patch isn't going to leak the global?  (I honestly don't know.)

Out of my league, will defer to experts :).
Attachment #718820 - Flags: feedback?(jones.chris.g) → feedback?(justin.lebar+bug)
Comment on attachment 718820 [details] [diff] [review]
use addSystemEventListener to receive all touch events without affecting by stopPropagation.

r+ But I would like to heard from Justin about leaking the global (I tend to think that this will be freed when the process will be killed in the worst case ?)
Attachment #718820 - Flags: review?(21) → review+
We use the same idiom in BrowserElementChildPreload, so I don't see how this could be bad if that code is fine.  And hopefully we'd have noticed by now if that code was leaking.
Attachment #718820 - Flags: feedback?(justin.lebar+bug) → feedback+
QA Contact: ahubenya
https://hg.mozilla.org/mozilla-central/rev/8f3d3cdc8632
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
This issue is verified as fixed. The application scrolls now.
Build: 20130301030548 
Gecko: http://hg.mozilla.org/mozilla-central/rev/993d7aff3109
Gaia: 7f0a1d5341ac8870e70f64f543ec79a729bf717e
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.