Closed
Bug 1146843
Opened 9 years ago
Closed 9 years ago
Tapping on seek bar in videos will open the context menu
Categories
(Firefox for Android Graveyard :: Toolbar, defect)
Tracking
(firefox36 unaffected, firefox37 unaffected, firefox38 unaffected, firefox39+ fixed, firefox40+ verified, fennec39+)
VERIFIED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox36 | --- | unaffected |
firefox37 | --- | unaffected |
firefox38 | --- | unaffected |
firefox39 | + | fixed |
firefox40 | + | verified |
fennec | 39+ | --- |
People
(Reporter: cos_flaviu, Assigned: kats)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
10.01 KB,
patch
|
snorp
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Environment: Device: Asus Transformer Tab (Android 4.2.1); Build: Nightly 39.0a1 (2015-03-23); Steps to reproduce: 1. Load a video (e.g.: http://people.mozilla.com/~nhirata/html_tp/elephants-dream.webm); 2. Tap anywhere on the seek bar. Expected result: The video jumps to that specific time. Actual result: The context menu is displayed.
Reporter | ||
Comment 1•9 years ago
|
||
Regression range: Last good build: 2015-03-19 First bad build: 2015-03-20 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cf1060d8ce9f&tochange=4d2d97b3ba34
Keywords: regression
Updated•9 years ago
|
tracking-fennec: --- → ?
Comment 2•9 years ago
|
||
Flaviu - Any chance you could get a regression range in fx-team or inbound to narrow things down?
Comment 3•9 years ago
|
||
bug 1142336 looks intresting
Reporter | ||
Comment 4•9 years ago
|
||
(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 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=01abf38a6a4d&tochange=83c5d1c58506
Comment 5•9 years ago
|
||
[Tracking Requested - why for this release]: regression from Bug 1144650 or Bug 1144324, both Kats ni to look at this bug.
Assignee | ||
Updated•9 years ago
|
Blocks: 1144324
Component: Audio/Video → Graphics, Panning and Zooming
Flags: needinfo?(bugmail.mozilla)
Assignee | ||
Comment 6•9 years ago
|
||
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. [1] https://hg.mozilla.org/integration/mozilla-inbound/rev/33c30e283fa8#l3.49
Assignee | ||
Comment 7•9 years ago
|
||
The critical change here is setting mHoldInQueue to true instead of mWaitForTouchListeners in TouchEventHandler. All the rest is just code removal. https://treeherder.mozilla.org/#/jobs?repo=try&revision=0626102adbe1
Attachment #8585086 -
Flags: review?(snorp)
Assignee | ||
Updated•9 years ago
|
status-firefox40:
--- → affected
Comment on attachment 8585086 [details] [diff] [review] Patch 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-
Comment 9•9 years ago
|
||
Tracking for 39+ since this is a recent regression.
Assignee | ||
Comment 10•9 years ago
|
||
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)
Updated•9 years ago
|
tracking-fennec: ? → 39+
Attachment #8587286 -
Flags: review?(snorp) → review+
Assignee | ||
Comment 11•9 years ago
|
||
Try https://treeherder.mozilla.org/#/jobs?repo=try&revision=d244b7c68be6 includes this change and looks green. Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/19d96ad34466
Assignee | ||
Comment 12•9 years ago
|
||
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?
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/19d96ad34466
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Reporter | ||
Comment 14•9 years ago
|
||
Verified as fixed in build 40.0a1 2015-04-06; Device: Asus Transformer Tab (Android 4.2.1).
Comment 16•9 years ago
|
||
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+
Updated•6 years ago
|
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•