Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

unspecified
2.2 S6 (20feb)
x86
macOS

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [systemsfe])

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
We see the following error: [JavaScript Error: "too much recursion"]

I think this might have something to do with trying to drag/drop a place holder, or single app within a group. Working on narrowing this down.
Comment hidden (obsolete)
(Assignee)

Comment 2

4 years ago
Probably want to block as this makes the home screen freeze.
blocking-b2g: --- → 2.2?
Whiteboard: [systemsfe]
(Assignee)

Comment 3

4 years ago
Found an STR:

1 - Have a group at the top of the screen with only 1 icon.
2 - Long press the icon to enter edit mode and release.
3 - Long press and let go on an empty place after the first icon (a placeholder).
4 - Try to long-press on any icon, I think we are inside an infinite loop at this point.
(Assignee)

Updated

4 years ago
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
(Assignee)

Comment 5

4 years ago
Comment on attachment 8563318 [details] [review]
[gaia] KevinGrandon:bug_1132391_placeholder_pointer_events > mozilla-b2g:master

Chris - you're right, after finding the STR it was a pretty simple fix.

I spent quite a long time trying to write a test for this, but ultimately failed. Following the STR in a test is quite easy, but I don't think we have good methods to easily follow the same path as our touch events do. If you have any ideas let me know, but I think we might just want to land this without a test. I don't think it would easily regress.
Attachment #8563318 - Flags: review?(chrislord.net)
(Assignee)

Comment 6

4 years ago
This also allows users to long-press on a placeholder to move the entire group which I think is a nice benefit.
Comment on attachment 8563318 [details] [review]
[gaia] KevinGrandon:bug_1132391_placeholder_pointer_events > mozilla-b2g:master

This is a conditional r+, because:

I think this rule will add weird behaviour if you long-press on a placeholder in a large group - the group would then collapse, but be very far from the user's finger...

So this is an r+ on the condition that you disable placeholders triggering group moves (I think a CSS rule that disabled pointer events on not:(.collapsed) group backgrounds/shadows would probably do the trick - likely there's already a rule that can be modified).
Attachment #8563318 - Flags: review?(chrislord.net) → review+
(Assignee)

Comment 8

4 years ago
Thanks. I've added a new selector to handle the existing pointer events rule for a lot of the grouping stuff. Starting the drag from the bottom of a large group did indeed screw up the experience.

In master: https://github.com/mozilla-b2g/gaia/commit/9fe2b6583206cea52656b0d00941684ecfc58e42
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

4 years ago
Comment on attachment 8563318 [details] [review]
[gaia] KevinGrandon:bug_1132391_placeholder_pointer_events > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Unsure, this likely group with some grouping work.
[User impact] if declined: The app can freeze, causing it to restart.
[Testing completed]: Manual testing - currently working on adding an integration test for this, but it's extremely difficult.
[Risk to taking this patch] (and alternatives if risky): Low risk as it's a simple CSS-only patch.
[String changes made]: No
Attachment #8563318 - Flags: approval-gaia-v2.2?(bbajaj)
blocking-b2g: 2.2? → 2.2+
Target Milestone: --- → 2.2 S6 (20feb)
Reverted because it was my best guess for what caused the Gu near-permafail starting on the below Bumper Bot push:
https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=bd459caf1f9a

Master: https://github.com/mozilla-b2g/gaia/commit/3c6d38bb0b016eb8d1083a8f3c3d837303a71dda

https://treeherder.mozilla.org/logviewer.html#?job_id=1344176&repo=b2g-inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 11

4 years ago
Hmm, this seems to be intermittent on b-i somehow? Will investigate tomorrow.
(Assignee)

Comment 13

4 years ago
Comment on attachment 8563691 [details] [review]
[gaia](reland) KevinGrandon:bug_1132391_reland > mozilla-b2g:master

This patch is to reland when/if things turn green.
Attachment #8563691 - Attachment description: [gaia] KevinGrandon:bug_1132391_reland > mozilla-b2g:master → [gaia](reland) KevinGrandon:bug_1132391_reland > mozilla-b2g:master
waiting for re-landing here before taking it on the branch.
*sigh* wasn't yours, so I'm back to the drawing board
Master: https://github.com/mozilla-b2g/gaia/commit/237525eddabc234e886cea22b51ddcd1504207dc
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Attachment #8563318 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
cannot find the error logs [JavaScript Error: "too much recursion"]

*2.2
Build ID               20150326164141
Gaia Revision          6d0174e28576f2f93e696a43d1ac3b03340117f6
Gaia Date              2015-03-26 21:47:33
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/820c2f2e817a
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150326.201153
Firmware Date          Thu Mar 26 20:12:05 EDT 2015
Bootloader             L1TC000118D0

*master
Build ID               20150326160206
Gaia Revision          525c341254e08f07f90da57a4d1cd5971a3cc668
Gaia Date              2015-03-26 16:34:16
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/59554288b4eb
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150326.193247
Firmware Date          Thu Mar 26 19:32:58 EDT 2015
Bootloader             L1TC100118D0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.