Martin Shuman [:Marty] Reporter
	Description • 10 years ago

Description:
Tapping on the text for 'Silence/Ring incoming calls,' 'Restart,' or 'Power off' will not activate that function.  The user must tap the blank space next to the text in that list item to activate the function.

Note: This issue does NOT occur when tapping the text of the first list item, 'Turn on/off airplane mode.'  Tapping that text has the correct functionality.

Repro Steps:
1) Update a Flame to 20150203055641
2) Long-Press the power button to bring up the Power Menu
3) Select 'Silence incoming calls' by tapping directly on the text.
4) Select 'Silence incoming calls' by tapping in the blank space next to the text.

Actual:
Tapping on the text does not select the menu item, while tapping next to the text does select the item.

Expected:
The menu item is selected when tapping the text.

Environmental Variables:
Device: Flame 3.0 (Full Flash)(319MB)
Build ID: 20150203055641
Gaia: ae5a1580da948c3b9f93528146b007fc4f6a712b
Gecko: ae5d04409cd9
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0


Repro frequency: 7/8
See attached: video clip (URL), logcat

-------------------------------

This issue does NOT occur on Flame 2.2.
The menu item is selected when tapping the text.

Environmental Variables:
Device: Flame 2.2 (Full Flash)(319MB)
Build ID: 20150203002504
Gaia: cd62ff9fe199fb43920ba27bd5fdbc5c311016fc
Gecko: 11d93135c678
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

	Peter Bylenga [:PBylenga]
	Comment 1 • 10 years ago

[Blocking Requested - why for this release]:
Functional regression of a core feature.

Requesting a window.

	Gabriela [:gaby2300]
	Comment 2 • 10 years ago

I can reproduce this bug in a Flame device with the same 3.0 build but using base image v188.

Repro frequency: 5/5

	Jayme Mercado [:JMercado] (Inactive 3/4/2016)
	Comment 3 • 10 years ago

Bug 950934 seems to have caused this issue.

Mozilla-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150201170935
Gaia: 740c7c2330d08eb9298597e0455f53d4619bbc1a
Gecko: 231a8c61b49f
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

First Broken 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150201174135
Gaia: 740c7c2330d08eb9298597e0455f53d4619bbc1a
Gecko: bcefc7d8d885
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Last Working gaia / First Broken gecko - Issue DOES occur
Gaia: 740c7c2330d08eb9298597e0455f53d4619bbc1a
Gecko: bcefc7d8d885

First Broken gaia / Last Working gecko - Issue does NOT occur 
Gaia: 740c7c2330d08eb9298597e0455f53d4619bbc1a
Gecko: 231a8c61b49f

Gecko Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=231a8c61b49f&tochange=bcefc7d8d885

	KTucker [:KTucker][Inactive 3/4/2016]
	Comment 4 • 10 years ago

Botond, can you take a look at this please. This might have been caused by the work done on bug 950934.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 5 • 10 years ago

The logcat contains the error "TypeError: evt.target.dataset is undefined" when this happens. Inspecting in WebIDE indicates that evt.target is a text node rather than the <li> element containing the text, which is odd. Not sure why bug 950934 would have caused the click event to get generated with a bad target.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 6 • 10 years ago

If I add a mousedown listener in sleep_menu.js it gets the <li> element as the target, so the bad target is specific to the click event.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 7 • 10 years ago

After a bunch of tracing through code it seems like the mCurrentTarget pointer inside the EventStateManager gets changed while in the process of dispatching the mouse-up event, and so the click event ends up with a bad target. As best I can tell the attached code is what's fiddling with the mCurrentTarget.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 8 • 10 years ago

Here's another stack that messes with the ESM state. It's not clear to me why sometimes we hit one and sometimes we hit the other.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 9 • 10 years ago

Even with both of these removed there's something screwing up. Falling down the rabbit hole...

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 10 • 10 years ago

Ok, I think I figured out what's going on. For the mouse-up event, the "current frame" is aec4bed8 and code at [1] finds the correct containing element node to dispatch the event to. During processing of the mouse-up event, it looks like the frame is removed from the frame tree due to some restyling happening. I'm not sure of the details there, but the code at [2] runs and sets the mCurrentEventContent to aec12680 as expected. Then, the mouse-up's post-handler runs from [3]. Note that one of the arguments to this function is GetCurrentEventFrame(), which in this instance returns aec4c018 which is actually the :before text frame for the node. I have no idea why this happens and I asked on #layout but nobody has responded yet. Finally, when the click event is dispatched it calls GetEventTargetContent at [4] and dispatches to that. Unfortunately here the target content is computed from the :before text frame and the target content ends up being a #text node. There is no code that ensures we walk up to the containing element, and that's what seems to be the problem.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?rev=5af5c02018a2#7934
[2] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?rev=5af5c02018a2#2178
[3] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?rev=5af5c02018a2#8257
[4] http://mxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp?rev=5af5c02018a2#4439

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 12 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ceae3faa925

	Carsten Book [:Tomcat]
	Comment 13 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/4ceae3faa925

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 14 • 10 years ago

Comment on attachment 8559333 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: unknown
[User impact if declined]: in some cases it is possible for click events to be dispatched to web content with a #text node as the target, in violation of specifications. this specifically causes a user-visible bug on B2G but technically affects all platforms which is why i'm requesting aurora approval instead of just b2g37. this bug has existed for a long time as far as I can tell, so I don't consider it important enough to request beta approval.
[Describe test coverage new/current, TreeHerder]: tested in the b2g scenario as described in this bug
[Risks and why]: fairly low risk; the code added is simple and will be a no-op except in the case that we care about. the code itself is a copy of code that already exists on the paths that other mouse events go through so in that respect it's well-tested
[String/UUID change made/needed]: none

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 15 • 10 years ago

Comment on attachment 8559333 [details] [diff] [review]
Patch

Although I'm not aware of reports of issues on Aurora, I think we should take this fix for platform consistency. Aurora+

	Ryan VanderMeulen [:RyanVM]
	Comment 16 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/54a1fe1e293b

	Ryan VanderMeulen [:RyanVM]
	Comment 17 • 10 years ago

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/54a1fe1e293b

	Jayme Mercado [:JMercado] (Inactive 3/4/2016)
	Comment 18 • 10 years ago

This issue is verified fixed on 3.0 Flame Nightly.  The current Flame 2.2 Nightly does not have this yet.  Ni? myself to verify that tomorrow.

Results:  Tapping all parts of the item triggers the list item including the text.

Environmental Variables:
Device: Flame 3.0
BuildID: 20150211010216
Gaia: 8c7865486a1b11076b849bbf8f7fccbaffbfafe7
Gecko: ee093ca70666
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

	Jayme Mercado [:JMercado] (Inactive 3/4/2016)
	Comment 19 • 10 years ago

This issue was not on 2.2 but the uplift did not seem to have any ill effects either.

Environmental Variables:
Device: Flame 2.2
BuildID: 20150212002504
Gaia: 791e53728cd8018f1d7cf7efe06bbeb1179f0370
Gecko: 5dec207fcbeb
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0