Closed Bug 1146843 Opened 7 years ago Closed 7 years ago

Tapping on seek bar in videos will open the context menu


(Firefox for Android Graveyard :: Toolbar, defect)

Not set


(firefox36 unaffected, firefox37 unaffected, firefox38 unaffected, firefox39+ fixed, firefox40+ verified, fennec39+)

Firefox 40
Tracking Status
firefox36 --- unaffected
firefox37 --- unaffected
firefox38 --- unaffected
firefox39 + fixed
firefox40 + verified
fennec 39+ ---


(Reporter: cos_flaviu, Assigned: kats)



(Keywords: regression)


(1 file, 1 obsolete file)

Device: Asus Transformer Tab (Android 4.2.1);
Build: Nightly 39.0a1 (2015-03-23);

Steps to reproduce:
1. Load a video (e.g.:;
2. Tap anywhere on the seek bar.

Expected result:
The video jumps to that specific time.

Actual result:
The context menu is displayed.
Regression range:

Last good build: 2015-03-19
First bad build: 2015-03-20

Keywords: regression
tracking-fennec: --- → ?
Flaviu - Any chance you could get a regression range in fx-team or inbound to narrow things down?
bug 1142336 looks intresting
(In reply to Mark Finkle (:mfinkle) from comment #2)
> Flaviu - Any chance you could get a regression range in fx-team or inbound
> to narrow things down?

Inbound regression window:

Last good revision: 01abf38a6a4d
First bad revision: 83c5d1c58506
[Tracking Requested - why for this release]: regression from Bug 1144650 or Bug 1144324, both Kats ni to look at this bug.
Assignee: nobody → bugmail.mozilla
Flags: needinfo?(bugmail.mozilla)
Blocks: 1144324
Component: Audio/Video → Graphics, Panning and Zooming
Flags: needinfo?(bugmail.mozilla)
In bug 1144324 I removed a bunch of code including [1] which in hindsight was a little too much. However I'd rather fix this by ripping out even more code than by putting it back. This basically disables a small optimization which makes the Java pan/zoom code not wait for touch listeners (i.e. now it will always wait for initial touch event dispatch, even if there are no touch listeners on the page). I don't expect there to be much perf impact from this since in the case that there are no touch listeners dispatching a touch event will be super fast anyway, and it's a lot of code that we can remove. All this will be useless anyway once we switch to APZ since we have a different mechanism to deal with this stuff there.

Attached patch Patch (obsolete) — Splinter Review
The critical change here is setting mHoldInQueue to true instead of mWaitForTouchListeners in TouchEventHandler. All the rest is just code removal.
Attachment #8585086 - Flags: review?(snorp)
Comment on attachment 8585086 [details] [diff] [review]

Review of attachment 8585086 [details] [diff] [review]:

Kats and I talked on IRC, and my concern is that this forces us to wait on Gecko to judge every touch event. That means relying on the main thread replying in a reasonable amount of time, which will frequently be a problem. r- because of this.
Attachment #8585086 - Flags: review?(snorp) → review-
Tracking for 39+ since this is a recent regression.
Flags: qe-verify+
smaug's taking a break from reviews so I don't wanna ask him. It's a fairly straightforward change; partial backout of the part 2 patch from bug 1144324, just enough to get the observer notification firing again.
Attachment #8585086 - Attachment is obsolete: true
Attachment #8587286 - Flags: review?(snorp)
tracking-fennec: ? → 39+
Attachment #8587286 - Flags: review?(snorp) → review+
Comment on attachment 8587286 [details] [diff] [review]
Partial backout of bug 1144324 part 2

Approval Request Comment
[Feature/regressing bug #]: bug 1144324
[User impact if declined]: preventDefault notifications on touch events might get ignored, resulting in unexpected user-visible behavior. This bug is an example of such behavior but there are likely other effects as well.
[Describe test coverage new/current, TreeHerder]: tested locally
[Risks and why]: low risk, is a partial backout of over-aggressive code removal
[String/UUID change made/needed]: yes, had to bump NS_PIDOMWINDOW_IID
Attachment #8587286 - Flags: approval-mozilla-aurora?
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Verified as fixed in build 40.0a1 2015-04-06;
Device: Asus Transformer Tab (Android 4.2.1).
Duplicate of this bug: 1150193
Comment on attachment 8587286 [details] [diff] [review]
Partial backout of bug 1144324 part 2

Approving for uplift to 39; sounds like we need this fix, it's been verified, and has had some time to bake on Nightly.
Attachment #8587286 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.