Closed Bug 1144410 Opened 5 years ago Closed 4 years ago

[Settings][Call Barring] Trying to activate Call Barring twice with the wrong passcode will cause lower section of the screen to stop responding to touch input.

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.5?, firefox37 unaffected, firefox38 wontfix, firefox38.0.5 fixed, firefox39 fixed, firefox40 fixed, b2g-v2.2 unaffected, b2g-master verified)

RESOLVED FIXED
2.2 S11 (1may)
blocking-b2g 2.5?
Tracking Status
firefox37 --- unaffected
firefox38 --- wontfix
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed
b2g-v2.2 --- unaffected
b2g-master --- verified

People

(Reporter: Marty, Assigned: dbaron)

References

()

Details

(Keywords: regression, Whiteboard: [3.0-Daily-Testing])

Attachments

(2 files, 1 obsolete file)

Description:
If the user attempts to activate Call Barring with the wrong passcode, they will be presented with an error banner at the bottom of the screen.  Attempting to activate Call Barring again will result in the same error banner, but afterwards, the user will not be able to select anything in the Settings app where the error banner used to be.

Note:
-This deadzone for touch input seems to move up to remain above the keyboard.  If the user attempts to activate one of the call barring options again, they will be able to input a passcode with the keyboard, but will be unable to select Cancel or OK.

-This issue only occurs in the Settings app, and can be resolved by closing and relaunching the Settings app.

Repro Steps:
1) Update a Flame to 20150317073344
2) Open the Settings app and navigate to Call Settings > Call Barring.
3) Once the network information resolves, select the Incoming Calls All toggle.
4) Enter in the wrong Passcode and select OK
5) Repeat steps 3 and 4.
6) Select Incoming Calls All again, and attempt to select either the Cancel or OK buttons.

Actual:
The user cannot select the Cancel or OK buttons.

Expected:
The user is able to select UI elements in any area of the screen.

Environmental Variables:
Device: Flame 3.0 (319MB)(Full Flash)
Build ID: 20150317073344
Gaia: 738987bd80b0ddb4ccf853855388c2627e19dcc1
Gecko: 008b3f65a7e0
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Repro frequency: 5/5
See attached: logcat, video (URL)
This issue does NOT affect 2.2 builds.
The user is able to select UI elements in any area of the screen.

Environmental Variables:
Device: Flame 2.2 (319MB)(Full Flash)
Build ID: 20150316002502
Gaia: a6b2d3f8478ec250beb49950fecbb8a16465ff6f
Gecko: 18619f8f6c5c
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Keywords: regression
[Blocking Requested - why for this release]:
Functional regression that persists after leaving Call Barring area.

Requesting a window.
blocking-b2g: --- → 3.0?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
QA Contact: bzumwalt
Mozilla-Inbound Regression Window:

Last working Mozilla-Inbound build:
Device:  Flame 3.0
BuildID: 20150216141433
Gaia: ae02fbdeae77b2002cebe33c61aedeee4b9439fd
Gecko: 4b118b959ffd
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 Mozilla-Inbound build: 
Device: Flame 3.0
Build ID: 20150216142444
Gaia: ae02fbdeae77b2002cebe33c61aedeee4b9439fd
Gecko: 5b4a18aeb511
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0


Working Gaia with Broken Gecko issue DOES reproduce:
Gaia: ae02fbdeae77b2002cebe33c61aedeee4b9439fd
Gecko: 5b4a18aeb511

Working Gecko with Broken Gaia issue does NOT reproduce:
Gaia: ae02fbdeae77b2002cebe33c61aedeee4b9439fd
Gecko: 4b118b959ffd


Mozilla-Inbound Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4b118b959ffd&tochange=5b4a18aeb511


Issue appears to occur due to changes made in bug 960465
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
David, can you take a look at this please? This might be the result of the work done on bug 960465.
Blocks: 960465
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(dbaron)
Some behavior changes were expected from bug 960465, so it would be interesting to hear from whoever wrote the relevant gaia code what the gaia code here is doing in terms of transitions.
(In reply to Martin Shuman [:Marty] from comment #0)
> Repro Steps:
> 1) Update a Flame to 20150317073344
> 2) Open the Settings app and navigate to Call Settings > Call Barring.
> 3) Once the network information resolves, select the Incoming Calls All
> toggle.
> 4) Enter in the wrong Passcode and select OK

I can't even test these steps in any useful way because selecting OK (or Cancel) in step (4) does nothing, and I have to kill the settings app.
I guess I can see what happens with the exact revisions from comment 3, though not today, since it will take a long time to rebuild.
I can reproduce the bug with the gaia revision from comment 3 combined with current gecko.

Turning off off-main-thread animations doesn't affect it.
OK:

 * the first time through, the error has a transition on opacity from 1 to 0, 600ms

 * the second time through, there's no transition; it changes instantly.  I'm guessing there's also a transitionend event listener somewhere.
I'm not sure, but I suspect the problem is that we don't remove transitions from the set of completed transitions when an element that was previously display:none gets a new style (differing from the end value of completed transitions).
Yes, that's the problem, and I've confirmed that my patch fixes the issue using current gecko and ae02fbdeae77b2002cebe33c61aedeee4b9439fd gaia.
Flags: needinfo?(dbaron)
Since bug 960465 patch 14, we've retained finished transitions in order
to handle the issues described in
https://lists.w3.org/Archives/Public/www-style/2015Jan/0444.html .  The
code that did this made the assumption that the transition manager is
notified of the full sequence of style changes that happen to an
element.  However, when an element becomes part of a display:none
subtree and then later becomes displayed again, the transition manager
is not notified of a style change when it becomes displayed again (when
we do not have the old style context).

This patch introduces code to prune the finished transitions when that
happens.

This really fixes only part of the set of problems described in bug
1158431, which also affect running transitions.  However, it's the part
of that set that was a regression from bug 960465, which introduced the
retention of finished transitions, and which makes these issues
substantially easier to hit.

I'd like to fix this part quickly because it's a regression and we
should backport the fix.

Without the patch, I confirmed that the following two tests fail:
INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_dynamic_changes.html | bug 1144410 test - opacity after starting second transition - got 0, expected 1
INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_dynamic_changes.html | bug 1144410 test - opacity during second transition - got 0, expected 0.5

With the patch, all the added tests pass.
Attachment #8597563 - Flags: review?(bbirtles)
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
New try run, with the addition of code to destroy the collection if it's empty:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e801408bef5d
Since bug 960465 patch 14, we've retained finished transitions in order
to handle the issues described in
https://lists.w3.org/Archives/Public/www-style/2015Jan/0444.html .  The
code that did this made the assumption that the transition manager is
notified of the full sequence of style changes that happen to an
element.  However, when an element becomes part of a display:none
subtree and then later becomes displayed again, the transition manager
is not notified of a style change when it becomes displayed again (when
we do not have the old style context).

This patch introduces code to prune the finished transitions when that
happens.

This really fixes only part of the set of problems described in bug
1158431, which also affect running transitions.  However, it's the part
of that set that was a regression from bug 960465, which introduced the
retention of finished transitions, and which makes these issues
substantially easier to hit.

I'd like to fix this part quickly because it's a regression and we
should backport the fix.

Without the patch, I confirmed that the following two tests fail:
INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_dynamic_changes.html | bug 1144410 test - opacity after starting second transition - got 0, expected 1
INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_dynamic_changes.html | bug 1144410 test - opacity during second transition - got 0, expected 0.5

With the patch, all the added tests pass.
Attachment #8597572 - Flags: review?(bbirtles)
Attachment #8597563 - Attachment is obsolete: true
Attachment #8597563 - Flags: review?(bbirtles)
Comment on attachment 8597572 [details] [diff] [review]
Remove finished transitions when a frame transitions away from being display:none

>@@ -604,16 +606,69 @@ nsTransitionManager::ConsiderStartingTra
>   }
>   aElementTransitions->UpdateAnimationGeneration(mPresContext);
> 
>   *aStartedAny = true;
>   aWhichStarted->AddProperty(aProperty);
> }
> 
> void
>+nsTransitionManager::PruneCompletedTransitions(mozilla::dom::Element *aElement,
>+                                               nsCSSPseudoElements::Type
>+                                                 aPseudoType,
>+                                               nsStyleContext *aNewStyleContext)

* placement here.

>+  /**
>+   * When we're resolving style for an element that previously didn't have
>+   * style, we might have some old finished transitions for it, if,
>+   * say, it was display:none for a bit, but previously displayed.

Nit: 'for a while' might be more clear to people whose mother tongue is not English.

>+   *
>+   * This method removes any finished transitions that don't match the
>+   * new style.
>+   */
>+  void PruneCompletedTransitions(mozilla::dom::Element *aElement,
>+                                 nsCSSPseudoElements::Type aPseudoType,
>+                                 nsStyleContext *aNewStyleContext);

* placement here too.
Attachment #8597572 - Flags: review?(bbirtles) → review+
https://hg.mozilla.org/mozilla-central/rev/eff50867d7e1
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Comment on attachment 8597572 [details] [diff] [review]
Remove finished transitions when a frame transitions away from being display:none

Approval Request Comment
[Feature/regressing bug #]: bug 960165
[User impact if declined]: When a page uses a CSS transition on an element, makes the element display:none at the end of the transition (or later), later restores the element with a style other than the end of the transition (likely the same as the start of the transition), and then tries to repeat the transition (to the same end value), the transition will not run the second time.  This pattern showed up multiple places in Gaia, at it seems likely that it probably occurs somewhere in content on the Web as well.
[Describe test coverage new/current, TreeHerder]: Landed an automated test with the patch.  Appears this landing did make today's nightly.
[Risks and why]: I think it's pretty low risk.  It generally restores the behavior prior to bug 960465 for this case, and the code it adds is modeled on similar code elsewhere.
[String/UUID change made/needed]: No
Attachment #8597572 - Flags: approval-mozilla-beta?
Attachment #8597572 - Flags: approval-mozilla-aurora?
(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #19)
> [Feature/regressing bug #]: bug 960165

Er, that should have been bug 960465.
Comment on attachment 8597572 [details] [diff] [review]
Remove finished transitions when a frame transitions away from being display:none

Approved for uplift to aurora. Looks like we have good test coverage for this. It would be nice not to ship this regression but I leave the decision for beta to Sylvestre.
Attachment #8597572 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8597572 [details] [diff] [review]
Remove finished transitions when a frame transitions away from being display:none

It is a bit late for 38.0 but happy to take it for 38.0.5
Attachment #8597572 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1160374
Unable to verify the fix as the issue is blocked by bug 1160374. The user is unable to enter any passcode or get out of the passcode screen without force closing Settings app.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150430010201
Gaia: db8ea705c0fd1b1684807f5a8e837bb9a36a6f96
Gecko: 4b9b12c248dc
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
This bug has been verified as "pass" on the latest build of Flame KK master by the STR in comment 0.
Actual result: The user is able to select UI elements in any area of the screen.
See attchment: Flame KK master.3GP
Occurrence rate: 0/10.

Device: Flame KK master 512mb (pass)
Build ID               20151209150205
Gaia Revision          961528f4391668bc89ec0be14fa367cea099b588
Gaia Date              2015-12-08 18:11:20
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/b40ba117fa757861c9caa660ae989122718b494b
Gecko Version          45.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151209.182453
Firmware Date          Wed Dec  9 18:25:03 EST 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Flame KK v2.5 512mb (pass)
Build ID               20151209170211
Gaia Revision          7ca639a7bb0bacf27f548841c52617bfc0e3b21f
Gaia Date              2015-12-09 14:07:01
Gecko Revision         http://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/a35e8eb98969970d1af28b265bf99a9edd11e9c2
Gecko Version          44.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151209.161633
Firmware Date          Wed Dec  9 16:16:42 UTC 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.