Closed Bug 1146843 Opened 5 years ago Closed 5 years ago
Tapping on seek bar in videos will open the context menu
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.
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
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 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=01abf38a6a4d&tochange=83c5d1c58506
[Tracking Requested - why for this release]: regression from Bug 1144650 or Bug 1144324, both Kats ni to look at this bug.
5 years ago
Component: Audio/Video → Graphics, Panning and Zooming
In bug 1144324 I removed a bunch of code including  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.  https://hg.mozilla.org/integration/mozilla-inbound/rev/33c30e283fa8#l3.49
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)
5 years ago
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-
Tracking for 39+ since this is a recent regression.
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 #8587286 - Flags: review?(snorp) → review+
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
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?
Verified as fixed in build 40.0a1 2015-04-06; Device: Asus Transformer Tab (Android 4.2.1).
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+
You need to log in before you can comment on or make changes to this bug.