Closed
Bug 1131840
Opened 9 years ago
Closed 9 years ago
[Windows Management] - Tapping on the gray 'active alarm' banner at the top of the screen does not take the user to the active full-page alarm
Categories
(Core :: Layout, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox36 | --- | wontfix |
firefox37 | --- | wontfix |
firefox38 | --- | fixed |
b2g-v2.0 | --- | unaffected |
b2g-v2.1 | --- | unaffected |
b2g-v2.2 | --- | verified |
b2g-master | --- | verified |
People
(Reporter: jmitchell, Assigned: kats)
References
()
Details
(Keywords: regression, Whiteboard: [3.0-Daily-Testing])
Attachments
(5 files, 1 obsolete file)
231.99 KB,
text/plain
|
Details | |
79.42 KB,
text/plain
|
Details | |
25.07 KB,
patch
|
botond
:
review+
roc
:
review+
bajaj
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
3.03 KB,
patch
|
roc
:
review+
bajaj
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
7.61 KB,
patch
|
botond
:
review+
roc
:
review+
bajaj
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
Description: If you set an alarm to go off through the Clock app, and then hit the homescreen button when you receive the alarm notification you can minimize the notification and return to the homescreen. The full-page alarm notification will now be represented as a gray banner at the top of the screen. If you click on this notification banner you are not taken back to the full screen alarm notification. Repro Steps: 1) Update a Flame to 20150209010211 2) Open the Clock app 3) Select the Timer Tab 4) Set an alarm for 1 minute, 5) *Optional* set sound to no-sound, and turn vibrate off (for the sanity of your coworkers) 6) Hit Start 7) When you receive the notification hit the homescreen button 8) Tap the gray alarm banner at the top of the screen (if you miss the timer which goes away after a few seconds, it will return after 90 seconds or you can enlarge the alarm notification via the notification menu and re-minimize it using the homescreen button to prompt the banner again) Actual: Tapping on the banner does not take the user to the full screen alarm notification Expected: Tapping on the banner will take the user to the full screen alarm notification Environmental Variables: Device: Flame 3.0 Build ID: 20150209010211 Gaia: 0d7b35f23402c4cb29bca6b98280fec48a196dec Gecko: 3436787a82d0 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: 8/8 See attached: logcat, video: http://youtu.be/BWDiA3H_mCs ----------------------------------------------------------------------------------- This issue does NOT repro in 2.2, 2.1, or 2.0 Device: Flame 2.2 (KK - Nightly - Full Flash) Build ID: 20150209002504 Gaia: e827781324cbde91d2434b388f5dead3303a85ee Gecko: 0552759956d3 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 Device: Flame 2.2 (KK - Nightly - Full-Flashed) Build ID: 20150209002504 Gaia: e827781324cbde91d2434b388f5dead3303a85ee Gecko: 0552759956d3 Gonk: e7c90613521145db090dd24147afd5ceb5703190 Version: 37.0a2 (2.2) Firmware Version: v18D User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0 Device: Flame 2.1 (KK - Nightly - Full-Flashed) Build ID: 20150209001212 Gaia: 4a14bb118d55f3d15293c2ff55b7f29f9b0bfcdb Gecko: 6cbe28d0bb8c Gonk: e7c90613521145db090dd24147afd5ceb5703190 Version: 34.0 (2.1) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Device: Flame 2.0 (KK - Nightly - Full-Flashed) Build ID: 20150209000204 Gaia: 2989f2b2bd12fcc0e9c017d2db766e76a55873b8 Gecko: ad3cf982b94d Gonk: e7c90613521145db090dd24147afd5ceb5703190 Version: 32.0 (2.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Reporter | ||
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Comment 1•9 years ago
|
||
[Blocking Requested - why for this release]: Functional regression of a core feature. Requesting a window.
blocking-b2g: --- → 3.0?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Keywords: regressionwindow-wanted
Updated•9 years ago
|
QA Contact: pcheng
Mozilla-inbound Regression Window Last Working Environmental Variables: Gaia-Rev 0db8a38f9fed18ae2abf5ef7e1b6e2a570b07e0e Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/8b881bea204a Build-ID 20141223163030 Version 37.0a1 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20150210.050217 FW-Date Tue Feb 10 05:02:28 EST 2015 Bootloader L1TC000118D0 First Broken Environmental Variables: Gaia-Rev cb1dad4881533bff9f06d47e34983c7b10c04a8c Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/b17e7747d3fb Build-ID 20141223164836 Version 37.0a1 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20150210.050217 FW-Date Tue Feb 10 05:02:28 EST 2015 Bootloader L1TC000118D0 Last Working gaia / First Broken gecko - Issue does NOT occur Gaia: 0db8a38f9fed18ae2abf5ef7e1b6e2a570b07e0e Gecko: https://hg.mozilla.org/mozilla-central/rev/b17e7747d3fb First Broken gaia / Last Working gecko - Issue DOSE occur Gaia: cb1dad4881533bff9f06d47e34983c7b10c04a8c Gecko: https://hg.mozilla.org/mozilla-central/rev/8b881bea204a Gecko Pushlog: See log:logcat_flame_1532.txt
Keywords: regressionwindow-wanted
Comment 3•9 years ago
|
||
The above window is incorrect. The following is the correct window. mozilla-inbound regression window: Last Working Environmental Variables: Device: Flame BuildID: 20150201170935 Gaia: 740c7c2330d08eb9298597e0455f53d4619bbc1a Gecko: 231a8c61b49f Version: 38.0a1 (3.0 Master) 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 BuildID: 20150201174135 Gaia: 740c7c2330d08eb9298597e0455f53d4619bbc1a Gecko: bcefc7d8d885 Version: 38.0a1 (3.0 Master) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0 Gaia is the same so it's a Gecko issue. Gecko pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=231a8c61b49f&tochange=bcefc7d8d885 Caused by Bug 950934.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Comment 4•9 years ago
|
||
Botond, can you take a look at this please? Looks like the work done on Bug 950934 might have caused this to occur?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(botond)
Assignee | ||
Comment 5•9 years ago
|
||
I can do the initial investigation here as Botond is off today.
Assignee: nobody → bugmail.mozilla
Flags: needinfo?(botond)
Assignee | ||
Comment 6•9 years ago
|
||
I've been trying to catch this banner in WebIDE but I'm going around in circles and not getting anywhere. Marcus, do you know how I can (1) make this banner persistent so that it doesn't keep disappearing, and that I can debug it more easily and (2) find this banner in WebIDE? I think I found the code relating to it but the relevant .js files don't show up in the debugger view of either the system process or clock process, and when I browser around in the inspector I can't find the relevant elements either.
Flags: needinfo?(m)
Assignee | ||
Comment 7•9 years ago
|
||
Ok, after digging around some more I figured out how this is handled. There's showing/hiding of the banner is controlled by the timeouts in attention_toaster.js in the system process. The banner it self is shown in a 40px-high "attentionwindow" which actually loads the clock app's onring.html file inside a mozbrowser. The click listener is at [1]. What appears to be happening though is that the click listener is registered in the system process, and the click event is actually dispatched to the onring.html child process because that's what the APZ hit test returns. I haven't confirmed this last bit yet but it seems likely and would explain why this bug is happening. This might also need to be fixed on the gaia side because in general when you have an iframe and the user taps inside the iframe the containing page should not be receiving the click event; it's only the innermost window that receives it. [1] https://github.com/mozilla-b2g/gaia/blob/29ffd6441fef57ebe0c540f6d983ff4dc76d4ccf/apps/system/js/attention_window.js#L133
Flags: needinfo?(m)
Assignee | ||
Comment 8•9 years ago
|
||
So I thought I would fix this by setting pointer-events:none on the iframe that holds the onring.html so that the APZ would exclude that from the hit test. Turns out that iframe already has pointer-events:none set on it, and in the new APZ codepaths we're not respecting it properly. We handle the in-process case fine but this is a cross-process case (where pointer-events:none in the parent process encompasses the entire child process) and we don't handle that case. Working on a fix now.
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8564162 -
Flags: review?(roc)
Attachment #8564162 -
Flags: review?(botond)
Assignee | ||
Comment 10•9 years ago
|
||
This is to propagate the pointer-events:none from a parent process to a child process.
Attachment #8564164 -
Flags: review?(roc)
Attachment #8564164 -
Flags: review?(botond)
Assignee | ||
Comment 11•9 years ago
|
||
Since this will need uplifting to 2.2, I'd rather work around that bug than fix it properly right now.
Attachment #8564165 -
Flags: review?(roc)
Assignee | ||
Comment 12•9 years ago
|
||
Whoops, uploaded the wrong version last time.
Attachment #8564164 -
Attachment is obsolete: true
Attachment #8564164 -
Flags: review?(roc)
Attachment #8564164 -
Flags: review?(botond)
Attachment #8564168 -
Flags: review?(roc)
Attachment #8564168 -
Flags: review?(botond)
Comment 13•9 years ago
|
||
Comment on attachment 8564162 [details] [diff] [review] Part 1 - convert the forceDispatchToContent flag to an enum (no functional change) Review of attachment 8564162 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/apz/src/HitTestingTreeNode.h @@ +81,5 @@ > > void SetHitTestData(const EventRegions& aRegions, > const gfx::Matrix4x4& aTransform, > const Maybe<nsIntRegion>& aClipRegion, > + const EventRegionsOverride& aOverride); I generally prefer passing enums by value. @@ +125,5 @@ > * because we may use the composition bounds of the layer if the clip is not > * present. This value is in L's ParentLayerPixels. */ > Maybe<nsIntRegion> mClipRegion; > > + /* Indicates whether or not the event regions on this node needs to be s/needs/need ::: layout/ipc/RenderFrameParent.cpp @@ +636,5 @@ > int32_t appUnitsPerDevPixel = mFrame->PresContext()->AppUnitsPerDevPixel(); > nsIntRect visibleRect = GetVisibleRect().ToNearestPixels(appUnitsPerDevPixel); > visibleRect += aContainerParameters.mOffset; > nsRefPtr<Layer> layer = mRemoteFrame->BuildLayer(aBuilder, mFrame, aManager, visibleRect, this, aContainerParameters); > + layer->AsContainerLayer()->SetEventRegionsOverride(mEventRegionsOverride); RenderFrameParent::BuildLayer has some nullptr returns, so I think we should keep the null check.
Attachment #8564162 -
Flags: review?(botond) → review+
Comment 14•9 years ago
|
||
Comment on attachment 8564168 [details] [diff] [review] Part 2 - Add a flag to force an empty hit region Review of attachment 8564168 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/Layers.h @@ +921,5 @@ > + * as empty. Similarly, if there is a ForceDispatchToContent override then > + * the dispatch-to-content region must be treated as encompassing the entire > + * hit region, and therefore we must consult the content thread before > + * initiating a gesture. (If both flags are set, ForceEmptyHitRegion takes > + * priority.) Given that the flags are not meant to be used in combination, does it make sense to have them bit-flags as opposed to congiguous enum values? I guess future-proofing for a time when we'll have more flags and we might want to use some in combination? ::: layout/ipc/RenderFrameParent.cpp @@ +625,5 @@ > { > + if (aBuilder->IsBuildingLayerEventRegions()) { > + if (aBuilder->IsInsidePointerEventsNoneDoc() || > + aFrame->StyleVisibility()->GetEffectivePointerEvents(aFrame) == NS_STYLE_POINTER_EVENTS_NONE) { > + mEventRegionsOverride = (EventRegionsOverride)(mEventRegionsOverride | EventRegionsOverride::ForceEmptyHitRegion); This is an aside, but operators | and |= can be overloaded for enums: enum E { ... }; E operator|(E a, E b) { return (E)((int)a | (int)b); } E& operator|=(E& a, E b) { a = a | b; return a; }
Attachment #8564168 -
Flags: review?(botond) → review+
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #14) > Given that the flags are not meant to be used in combination, does it make > sense to have them bit-flags as opposed to congiguous enum values? I guess > future-proofing for a time when we'll have more flags and we might want to > use some in combination? I originally wrote this using sequential enums but that ended up a little messier. Plus conceptually the two flags are orthogonal and so the bitflags made more sense to me. And yes, future-proofing.. we might need more such flags for other regions that get added.
Attachment #8564162 -
Flags: review?(roc) → review+
Attachment #8564168 -
Flags: review?(roc) → review+
Attachment #8564165 -
Flags: review?(roc) → review+
Assignee | ||
Comment 16•9 years ago
|
||
failure |
Updated with review comments and pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd86ca9a6a68
Assignee | ||
Comment 17•9 years ago
|
||
Apparently "None" is already taken on Linux (probably #define'd to something). New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff32efc02a13
Comment 18•9 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #17) > Apparently "None" is already taken on Linux (probably #define'd to > something). By my reading of [1], "enum class", without any macros to wrap it, is fair game for 37, as long as you don't need to forward-declare it (which we don't, we just include LayersTypes.h). Any reason not to use it? [1] https://developer.mozilla.org/en-US/docs/Using_CXX_in_Mozilla_code
Assignee | ||
Comment 19•9 years ago
|
||
Unfortunately that doesn't actually solve the "None" problem, since None is #define'd to be 0L in some header that gets included into GLContextProviderGLX.cpp.
Assignee | ||
Comment 20•9 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/901326c8a66c remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/34a3bc7200c1 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/23c78316747f
Assignee | ||
Comment 21•9 years ago
|
||
Also note that this will affect 2.2 now that bug 950934 has been uplifted to 2.2.
Comment 22•9 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #19) > Unfortunately that doesn't actually solve the "None" problem, since None is > #define'd to be 0L in some header that gets included into > GLContextProviderGLX.cpp. Ah, true. (Yuck!)
Comment 23•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/901326c8a66c https://hg.mozilla.org/mozilla-central/rev/34a3bc7200c1 https://hg.mozilla.org/mozilla-central/rev/23c78316747f
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S6 (20feb)
Updated•9 years ago
|
blocking-b2g: 2.2? → 2.2+
Assignee | ||
Comment 24•9 years ago
|
||
Comment on attachment 8564162 [details] [diff] [review] Part 1 - convert the forceDispatchToContent flag to an enum (no functional change) NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 950934 User impact if declined: Setting pointer-events:none on an element in the parent process doesn't carry over to any child process content contained inside it. This manifests as being unable to tap on notification banners at the top of the screen such as the one for an alarm going off. Testing completed: locally Risk to taking this patch (and alternatives if risky): the change looks complicated but most of it is refactoring. it is a logical extension of the work done in bug 1125422 which was already uplifted. String or UUID changes made by this patch: none
Attachment #8564162 -
Flags: approval-mozilla-b2g37?
Assignee | ||
Updated•9 years ago
|
Attachment #8564165 -
Flags: approval-mozilla-b2g37?
Assignee | ||
Updated•9 years ago
|
Attachment #8564168 -
Flags: approval-mozilla-b2g37?
Assignee | ||
Updated•9 years ago
|
Component: Gaia::System::Window Mgmt → Layout
Product: Firefox OS → Core
Target Milestone: 2.2 S6 (20feb) → mozilla38
Version: unspecified → Trunk
Comment 25•9 years ago
|
||
Ktucker, can we please get thsi verified on today's 3.0 nightly ? Thanks!
Flags: needinfo?(ktucker)
Keywords: verifyme
Comment 26•9 years ago
|
||
This issue is verified fixed on Flame 3.0 master nightly user build. Tapping on the minimized alarm banner on top correctly brings the alarm screen back to full screen. Device: Flame 3.0 (full flash, 319MB mem) BuildID: 20150220010206 Gaia: e4f7c67378e33e83f88d38ddb4a6c2cabf1423c3 Gecko: 1b4c5daa7b7a 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 Leaving verifyme tag for 2.2 uplifting.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Updated•9 years ago
|
Attachment #8564162 -
Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Updated•9 years ago
|
Attachment #8564165 -
Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Updated•9 years ago
|
Attachment #8564168 -
Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Comment 27•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6057d615cd22 https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/01959a3f1d51 https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/902b712b408b
Comment 28•9 years ago
|
||
I have verified this bug successfully on Flame 2.2 Device Info: Build ID 20150225162504 Gaia Revision e4bf968d5a7366e7bdc58f0fdba28b32e864bdf7 Gaia Date 2015-02-25 18:39:43 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/da6ac51f5113 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150225.200419 Firmware Date Wed Feb 25 20:04:31 EST 2015 Bootloader L1TC000118D0
You need to log in
before you can comment on or make changes to this bug.
Description
•