Closed Bug 950225 Opened 6 years ago Closed 6 years ago

[B2G][Calendar]When adding a Yahoo or Caldav account hidden buttons in the background can be pressed

Categories

(Core :: Layout, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
1.3 C3/1.4 S3(31jan)
blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.2 --- unaffected
b2g-v1.3 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: jschmitt, Assigned: kanru)

References

Details

(Keywords: regression, Whiteboard: burirun1.3-1)

Attachments

(2 files, 2 obsolete files)

Description:
Hidden buttons 'Weekly' 'Day' or the alarm selection of an event are being pressed if the user selects part of the page while adding a Yahoo or Caldav account.

Repro Steps:
1) Updated Buri to BuildID: 20131213004002
2) Open the Calendar App
3) Select the drawer icon
4) Select the gear icon
5) Select the '+' icon
6) Select Caldav
7) While on the Caldav page, tap near the bottom right corner

Buri 1.3
Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20131213004002
Gaia: 888f9df5515a47d2f5806efee77485e05e1e5416
Gecko: dfae9c83bfbc
Version: 28.0a2
RIL Version: 01.02.00.019.102
Firmware Version: v1.2_20131115
Can you get a video?
Does not occur on Buri 1.2 MOZ.

Environmental Variables:
Device: Buri 1.2 MOZ
BuildID: 20131213004002
Gaia: 1aca7c4860e39b1a9969807d335dcf9f070ea9b3
Gecko: a2b69b561d9b
Version: 26.0
RIL Version: 01.02.00.019.102
Firmware Version: V1.2_20131115

Video - http://youtu.be/iKSzRr-Mb-8
QA Contact: gbennett
Here is a repro video using the latest 1.3 mozRIL: http://www.youtube.com/watch?v=eJun6lhK08M

Environmental Variables:
Device: Buri 1.3 mozRIL
BuildID: 20131213004002
Gaia: 888f9df5515a47d2f5806efee77485e05e1e5416
Gecko: dfae9c83bfbc
Version: 28.0a2
RIL Version: 01.02.00.019.102
Firmware Version: 20131115
Keywords: qawanted
How many places on that caldav screen can there be hidden buttons triggered?
Keywords: qawanted
Here is a video showing all the hidden buttons within caldav and yahoo that can be pressed: http://www.youtube.com/watch?v=MzSRprh2tl4
Keywords: qawanted
There's something really broken here.
blocking-b2g: --- → 1.3?
triage: 1.3+ for regression
blocking-b2g: 1.3? → 1.3+
I could not replicate the bug inside b2g-bin or inside firefox nightly (I still don't have a device to test).

Does anyone have any idea of what could be causing the problem? For me this bug doesn't make sense (looking at code from v1.3 branch), the `section#modify-account-view` has `z-index:1200` and is supposed to block ALL the touch/click interactions of the layers below it *unless* it has `pointer-events:none` (which isn't the case).

Can anyone confirm that this bug still happens with code on v1.3 branch? (any commit from this week) If that is the case this bug might be related with the way Gecko is interpreting the layers, which I wouldn't be able to fix by myself and don't even know who should I ask for help.

PS: one "hack" that would probably fix the problem is to set `pointer-events:none` to the `section#time-views` every time a `.fullscreen-view` is opened - which I wouldn't recommend since it would just beget more hacks in the future and probably mask the real bug.
QA Wanted - Can we confirm this is still happening on the latest 1.3 build?
Keywords: qawanted
This issue repros on the latest 1.3

Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140109004002
Gaia: 22bc6be5b76cdc6d4e9667ff070979041a20ce2f
Gecko: 2c8f8683bd0d
Version: 28.0a2
Firmware Version: v1.2_20131115
Keywords: qawanted
Here are my findings 

This issue is not replicable on latest gaia master with gecko as 1.2

Environment
OS Version 1.2.0.0. -prerelease
Platform: 26.0
BuildId: 20140109004002
Gecko : - http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/ec909be2a849

Updated with latest Gaia Master: 61797fd7cf908f13f140b98f94ace038e2321c05
using make reset-gaia

It appears to be a gecko issue which is replicable in gecko 1.3 and 1.4 irrespective
of gaia version

Kevin, I am not familiar with calendar app. Can you pl. validate the above findings and help
assign to the right team.
Thanks
Flags: needinfo?(kgrandon)
Assignee: nobody → evanxd
I can reproduce this easily on device (Geeksphone Peak) with today's 1.4 build, pressing in the lower right hand corner on the "Add an account" screen.
I am also able to reproduce this, but unfortunately I will unlikely have cycles to do serious debugging for the next 1-2 weeks.
I am going to clear the ni? flag for now, and can take another look in the future if needed.
Flags: needinfo?(kgrandon)
Evan,

There were only a handful of commits to calendar in 1.3, so it might be useful to bisect through those and also check to see if this reproduces on gecko 26 because it could also be a platform bug.
Flags: needinfo?(evanxd)
Hi Gareth,

The bug is also happened in Gaia master branch on gecko 29.
So, yes, it is a platform bug.

And the gecko 26 is used in v1.2.
Do we need to fix the bug for v1.2?
(We could see the mapping list in https://wiki.mozilla.org/Release_Management/B2G_Landing#Versions_and_Scheduling)

Thanks.
Flags: needinfo?(evanxd)
Attached file Pull request
Hi Gareth,

We do workaround here.
Because the click event just pass through the "create-account-view" panel to the under panel
with unknown reasons, then the bug is happened.
It might be a Gecko issue.

And the bug is just happened in real device not b2g-desktop or nightly.
So we do need to have marionette tests for it on b2g-desktop.

If we want to fix the issue without any workaround, we might fix it in Gecko not in Gaia.
But I think we could fix it in workaround way, and file a follow up bug for the possible gecko issue.

So please help me review the patch.
Thanks.
Attachment #8360324 - Flags: review?(gaye)
Comment on attachment 8360324 [details] [review]
Pull request

Hi Kevin,

Could you have time to give me feedback for the patch?
Thanks.

Evan
Attachment #8360324 - Flags: feedback?(kgrandon)
Comment on attachment 8360324 [details] [review]
Pull request

I generally hate workarounds, so I'm hesitant to provide an R+ or F+ unless we understand what platform issue is causing this. My recommendation would be to needsinfo a few gecko developers to see if they would know. 

Thank you for submitting a patch with a workaround though! It is really nice to have. My preference would be to just wait until we understand *what* is causing this, then land. But maybe Gareth will want to land anyway :)
Attachment #8360324 - Flags: feedback?(kgrandon)
Hi Kevin,

Thanks for the response.

Yes, we could cc Kanru a gecko guy to figure out the issue.
And thank Kanru.
Hi Kanru,

The event target is the element under "create-account-view" panel.
(We click on right side on "create-account-view" panel)

The click event is just pass through "create-account-view" panel to the under panel.
Do you have any idea for this?
Thanks.
Flags: needinfo?(kchen)
I don't know enough about the DOM stuff going on in calendar, but for e-mail in bug 960290 it appears that the user is able to click buttons on cards that are off-screen to the right/left, presumably because of click area inflation.  Are the things being clicked actually under the visible card, or are they off to the side?
Hm, and it appears to have been an 0.5px offset in e-mail though.  So maybe it's something different.  Well, something to consider!
(In reply to Andrew Sutherland (:asuth) from comment #23)
> Hm, and it appears to have been an 0.5px offset in e-mail though.  So maybe
> it's something different.  Well, something to consider!

I wonder if these are being caused by some gaia change to support edge gestures or something? I also noticed that it's possible to dismiss the keyboard by tapping at the bottom of the screen. I wonder if we can narrow down the root cause and avoid any workarounds. I may be able to look in the next few days..
See Also: → 960290
Vivien - any ideas?
Flags: needinfo?(21)
I'm not sure the workaround is what we want here (for calendar that is). If that was the solution, then we would be able to click the frame underneath anywhere on the account create screen. I think edge gestures is good intuition. Let's see if we can figure out where this was introduced and go from there.
Comment on attachment 8360324 [details] [review]
Pull request

^^
Attachment #8360324 - Flags: review?(gaye)
Hi all,

I disabled the edge gestures before, but the bug was still happened.
So I think the bug is not related with edge gestures.

I also tried to let create-account-view have 120% width, then the bug is not happened.
(In reply to Andrew Sutherland (:asuth) from comment #23)
> Hm, and it appears to have been an 0.5px offset in e-mail though.  So maybe
> it's something different.  Well, something to consider!

Hi Andrew,

Thanks for the tips.

I used the app manager to figure out the width of create-account-view and the under panel in device. The width of them are the same as 320px.

And the bug just could be reproduced in real device not in nightly or b2g-desktop.
And as I know, there is no 0.5px things in Calendar app.
But it is really a way to consider.
Really thanks.
PM Triaged, considered a blocker for 1.3
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Investigating.
Assignee: evanxd → kchen
Flags: needinfo?(kchen)
Reproduced on master

Gaia      c75a943bb4c2ca329c01406211c4fe77186d49cb
Gecko     7d855479045ad673d20cb398391542734b3ff5f7
BuildID   20140116120359
Version   29.0a1
ro.build.version.incremental=eng.cltbld.20131108.071732
ro.build.date=Fri Nov  8 07:39:33 EST 2013
Also reproduced on b2g-desktop of the same gecko/gaia revision.
The issue is caused by hit-target fluffing. Try set ui.touch.radius.enabled in b2g.js to false and the issue disappear. The hit test selected the calender view because it was not hidden and have event listener bound to it. You could workaround this by hide the calender view if it is covered by something else.

To fix it properly I think we should ignore the region outside of the viewport when doing hit-target fluffing.
Flags: needinfo?(21)
Attachment #8362528 - Flags: review?(roc)
Component: Gaia::Calendar → Layout
Product: Firefox OS → Core
Add test case.

I don't know a better way to get the bound rect of the document :/
Attachment #8362528 - Attachment is obsolete: true
Attachment #8362528 - Flags: review?(roc)
Attachment #8364846 - Flags: review?(roc)
Comment on attachment 8364846 [details] [diff] [review]
Restrict event fluffing in visible area with test case

Review of attachment 8364846 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/PositionedEventTargeting.cpp
@@ +264,4 @@
>               AppUnitsFromMM(aRootFrame, aPrefs->mSideRadii[3], false));
>    nsRect r(aPointRelativeToRootFrame, nsSize(0,0));
>    r.Inflate(m);
> +  return MoveInsideVisibleArea(aRootFrame, aRestrictToDescendants, r);

Let's call this ClipToFrame, and just intersect r with the bounds of aRestrictToDescendants (converted to the correct coordinate system using nsLayoutUtils::TransformFrameRectToAncestor, not GetOffsetToCrossDoc which won't take transforms into account properly). We know that aRootFrame is an ancestor of restrictToDescendants.
Attachment #8364846 - Flags: review?(roc) → review-
Comment on attachment 8365766 [details] [diff] [review]
Restrict event fluffing in visible area with test case v2.

Review of attachment 8365766 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/PositionedEventTargeting.cpp
@@ +231,5 @@
> + * Clip aRect with the bounds of aFrame in the coordinate system of
> + * aRootFrame. aRootFrame is an ancestor of aFrame.
> + */
> +static nsRect
> +ClipToFrame(nsIFrame* aRootFrame, nsIFrame* aFrame, nsRect aRect)

const nsRect& aRect

@@ +234,5 @@
> +static nsRect
> +ClipToFrame(nsIFrame* aRootFrame, nsIFrame* aFrame, nsRect aRect)
> +{
> +  nsRegion bound = nsLayoutUtils::TransformFrameRectToAncestor(
> +    aFrame, aFrame->GetRect(), aRootFrame);

aFrame->GetRect()'s top-left is not 0,0. So here you need nsRect(nsPoint(0, 0), aFrame->GetSize()).
Attachment #8365766 - Flags: review?(roc) → review+
Comment on attachment 8365766 [details] [diff] [review]
Restrict event fluffing in visible area with test case v2.

>+ClipToFrame(nsIFrame* aRootFrame, nsIFrame* aFrame, nsRect aRect)
>+{
>+  nsRegion bound = nsLayoutUtils::TransformFrameRectToAncestor(
>+    aFrame, aFrame->GetRect(), aRootFrame);
>+  nsRegion result = bound.Intersect(aRect);
>+  return result.GetBounds();
>+}

When this relands you should change nsRegion to nsRect here. TransformFrameRectToAncestor returns a rect, not a region.
The mochitest test runner created a iframe with width 500 which is wider than the browser window so the test couldn't simulate a mouse click in that region.
https://tbpl.mozilla.org/?tree=Try&rev=6c1d3aa80615

Disable the two problemtic test when the window is too narrow.
https://hg.mozilla.org/mozilla-central/rev/192c0bc86b01
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
This was backed out from Aurora because test_reftests_with_caret.html went perma-fail on Win7/Win8 after it landed. Funny enough, I'd filed bug 965526 earlier today for the exact same failure on trunkand Ehsan had already landed a fix on inbound for it. And indeed, it fixed the perma-fail here as well, so re-landed :)

https://hg.mozilla.org/releases/mozilla-aurora/rev/60d7769f9d4c

If you're wondering, my *guess* is that the changes in test_event_target_radius.html (which runs in the same mochitest chunk as test_reftests_with_caret.html) somehow perturbed the other test into failing. In other words, I don't think this directly caused the failure but made an already-flaky test more failure-prone that it was before.
You need to log in before you can comment on or make changes to this bug.