Icons on grid do not re-arrange automatically when dragging over collections

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: amirn, Assigned: crdlc)

Tracking

({regression})

unspecified
ARM
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(blocking-b2g:1.3+, b2g-v1.3 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
See attached video
(Reporter)

Comment 1

5 years ago
When dragging a dock icon between 2 homescreen icons, the homescreens icons move in order to "make room" for the dragged icon but do not go back the their original position when dragging stops.

Video of the bug:
https://www.dropbox.com/s/86p6np1r2eh3u4m/MUTE_20131009_131500.mp4
(Reporter)

Updated

5 years ago
Flags: needinfo?(crdlc)
(Assignee)

Comment 2

5 years ago
what info do you want? There is a bug :D
Flags: needinfo?(crdlc)
(Assignee)

Comment 3

5 years ago
Regression from bug 910324. If we add display none to the gray circle it works fine so I have to investigate a bit more to fix this
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Keywords: regression
(Assignee)

Updated

5 years ago
Depends on: 910324
(Assignee)

Updated

5 years ago
Assignee: crdlc → nobody
No longer depends on: 910324
(Assignee)

Comment 4

5 years ago
This is failing because of the elementFromPoint method is returning an element [1][2] with pointer-events: none in latest Gecko. It works fine in Gecko 18.1.

[1] http://dev.w3.org/csswg/cssom-view/#extensions-to-the-document-interface
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/grid.css#L96

AFAIK according to the spec: "The elementFromPoint() method does not necessarily return the top-most painted element. For instance, an element can be excluded from being a target for hit testing by using the 'pointer-events' CSS property."

http://dev.w3.org/csswg/cssom-view/#extensions-to-the-document-interface

Or, did it change?
Component: Gaia::Homescreen → Builds
(Assignee)

Updated

5 years ago
Summary: Icons on grid do not re-arrange automatically when dragging a dock icon → Icons on grid do not re-arrange automatically when dragging over collections

Updated

5 years ago
Component: Builds → General
(Assignee)

Comment 5

5 years ago
Thanks Jason, I didn't have idea which component I should set :(
(Assignee)

Updated

5 years ago
Status: ASSIGNED → NEW
(Assignee)

Updated

5 years ago
Duplicate of this bug: 944596
(Assignee)

Updated

5 years ago
Blocks: 910302

Updated

5 years ago
blocking-b2g: --- → 1.3?
Whiteboard: [systemsfe]

Updated

5 years ago
Keywords: regressionwindow-wanted

Updated

5 years ago
QA Contact: jzimbrick

Comment 7

5 years ago
Regression Window:

Last Working Environmental Variables:
Device: Buri 1.3 Mozilla RIL
BuildID: 20130926112034
Gaia: 623e7be3a47dd33df1c64509e6ba0b26b0550b82
Gecko: 153aebb30387
Version: 27.0a1
Base Image: V1.2_20131115

First Broken Environmental Variables:
Device: Buri 1.3 Mozilla RIL
BuildID: 20130927040201
Gaia: 64ba02f7bbf70a1877ba9dad6889a17cd25b1d35
Gecko: e4cd2242cc7d
Version: 27.0a1
Base Image: V1.2_20131115
Keywords: regressionwindow-wanted
If it's on the Gaia side, this might be caused by bug 910334 or bug 920948. bug 920960 is also in the regression range, but that seems unlikely.

Updated

5 years ago
Component: General → DOM: Device Interfaces
Product: Firefox OS → Core
I get the feeling this one of those cases where a landing Gaia revealed a gecko bug.

Andrew - Can you suggest someone to investigate this who knows a lot about elementFromPoint?
Flags: needinfo?(overholt)
Whiteboard: [systemsfe]
(In reply to Jason Smith [:jsmith] from comment #10)
> I get the feeling this one of those cases where a landing Gaia revealed a
> gecko bug.
> 
> Andrew - Can you suggest someone to investigate this who knows a lot about
> elementFromPoint?

Boris shows up in blame output but nsDocument::ElementFromPoint hasn't been touched in a while.

Is there a reduced test case that manifests itself in Firefox desktop?

Boris, see comment #4 for more details here.
Component: DOM: Device Interfaces → DOM: CSS Object Model
Flags: needinfo?(overholt)
Flags: needinfo?(bzbarsky)
Flags: needinfo?
I mostly have blame for the webidlification.  All the real work is under nsLayoutUtils::GetFrameForPoint.

Clicking the orange square in this testcase:

data:text/html,<script>window.onclick = function(e) { alert(document.elementFromPoint(e.clientX, e.clientY)) }</script><div style="width: 100px; height: 100px; background: orange; pointer-events: none">

Correctly shows the body in desktop Firefox, so this is generally working as it should...

I too would love a testcase that actually reproduces the problem on desktop.  It doesn't even have to be all that minimal; I can reduce it myself.  But it does need to reproduce the elementFromPoint problem clearly.
Flags: needinfo?(crdlc)
Flags: needinfo?(bzbarsky)
Flags: needinfo?
Oops, I missed that the changes in bug 804631 and bug 837242 are in gecko 26 but not in gecko 18.

roc or tn may also have thoughts here.
(Assignee)

Comment 14

5 years ago
Hi,

  The problem is with the markup generated with the pseudo class :after. It was working fine in gecko 18

https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/homescreen.css#L102

Thanks

(In reply to Boris Zbarsky [:bz] from comment #12)
> I mostly have blame for the webidlification.  All the real work is under
> nsLayoutUtils::GetFrameForPoint.
> 
> Clicking the orange square in this testcase:
> 
> data:text/html,<script>window.onclick = function(e) {
> alert(document.elementFromPoint(e.clientX, e.clientY)) }</script><div
> style="width: 100px; height: 100px; background: orange; pointer-events:
> none">
> 
> Correctly shows the body in desktop Firefox, so this is generally working as
> it should...
> 
> I too would love a testcase that actually reproduces the problem on desktop.
> It doesn't even have to be all that minimal; I can reduce it myself.  But it
> does need to reproduce the elementFromPoint problem clearly.
Flags: needinfo?(crdlc)
Christian, can you please either describe more clearly what exactly is the problem with the CSS you link to or post a testcase that would make it clear?

Alternately, is there a time I can talk to you on IRC, skype, vidyo, or via some other real-time communications mechanism?
Flags: needinfo?(crdlc)
(Assignee)

Comment 16

5 years ago
Hi Boris,

   We have all icon children elements with a pointer-events: none [1]. So when we call to elementFromPoint method, images or labels or whatever inside <li> elements are transparent (you know what I want to know, they aren't consuming the event :) ) and the target always is the <li> parent.

   But the new container added with the pseudo class :after (https://developer.mozilla.org/en-US/docs/Web/CSS/::after) is consuming the event and if you ask for the elementFromPoint over this new children, the elementFromPoint says: the target is <div::before> instead of <li> in this case [2]

    According to the spec[3], elements with pointer-events none cannot be the target of elementFromPoint. Then div:after which is the last child of div should have pointer-events to none and it cannot be never the target of the elementFromPoint method.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/grid.css#L66
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/homescreen.css#L102
[3] http://dev.w3.org/csswg/cssom-view/#extensions-to-the-document-interface

PS:

This is an icon in markup

<li>
   <div> <-- Pointer event none
    <img/>
    <span>My App<span>
    <circle generated with :after>
   </div>
</li>

Thanks a lot
Flags: needinfo?(crdlc)
(Assignee)

Comment 17

5 years ago
Sorry I forgot to say that it was working fine in Gecko 18.1
Wait, are you actually getting the generated content as the return value from elementFromPoint?  Or are you clicking on the circle in your example in comment 16 and elementFromPoint is returning the div?

I tried to create a testcase based on your markup and styles and it doesn't show the problem you seem to be describing.  In fact, it acts identically in Fx18 and in current trunk Firefox, as far as I can tell.  I'll attach that testcase, just in case it behaves differently for you for some reason.
Created attachment 8342412 [details]
Testcase that tods NOT show the problem
(Assignee)

Comment 20

5 years ago
You are ok and I am wrong. I can see how it is working fine although the bug is not reproducible removing the :after element from the CSS so I want to ask who added it in the code. Thanks for the support Boris
Flags: needinfo?(evyatar)
(Assignee)

Comment 21

5 years ago
Hy Evyatar,

  I've just checked and the problem is the transform and transition, please delete them and check, 10x

https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/homescreen.css#L117
hi Cristian, what do you mean? The scale is set to 1 - how come that makes the event not bubble through?
Is there a way to fix this, or do we need to get rid of this transition?
Flags: needinfo?(evyatar)
(Assignee)

Comment 23

5 years ago
Hi, I don't know what is happening but without this transition it is working fine. Is it needed? Maybe we can remove it without problems
Hmm well the transition adds to the look and feel, but of course we can give it up if it's so problematic...
Flags: needinfo?(ran)
Does the transition work with an earlier Gecko?  Is the transition new:

That is:

1)  Was there a platform regression here, or is there just a change in the styles?
2)  And if the latter, then is the change in the styles showing a preexisting Gecko bug?
(Assignee)

Comment 26

5 years ago
(In reply to Boris Zbarsky [:bz] from comment #25)
> Does the transition work with an earlier Gecko?  Is the transition new:

I think so.. please Evyatar, double-check please :)

> 
> That is:
> 
> 1)  Was there a platform regression here, or is there just a change in the
> styles?

It was added a couple of months ago

> 2)  And if the latter, then is the change in the styles showing a
> preexisting Gecko bug?

But honestly I don't know if this transition is needed
Current (26):
 With transition: doesn't work
 Without: works well

Couldn't check on 18.1 since I can't make current gaia master to it - it fails to load.

As far as I remember when Cristian and I checked it - the same homescreen code that doesn't work on latest gecko, worked on 18.1, so we assumed it's a gecko issue.
Flags: needinfo?(ran)
Patryk, this technical issue might force us to disable the "grow" animation when dragging into a Collection as we won't have time to research this bug any further.
Is that acceptable?
Flags: needinfo?(padamczyk)
I would still dearly love someone to give me a way to reproduce the Gecko issue.
> Patryk, this technical issue might force us to disable the "grow" animation
> when dragging into a Collection as we won't have time to research this bug
> any further.
> Is that acceptable?

If we can't fix this for v.1.3 due to timing issues then disabling it temporary for v.1.3 is our only option, I don't love the solution to remove this effect as its one of the few whimsical elements we have in the system but I don't believe we should regress how icons work for a visual effect.
Is there no time post feature complete to fix this?
Flags: needinfo?(padamczyk)
If triage allows it, I would rather the Bug stay open till it's fixed with no workaround.
(Assignee)

Updated

5 years ago
Duplicate of this bug: 948652
(Assignee)

Comment 33

5 years ago
I would like to disable this effect right now because there are some bug appearing continuously on bugzilla related to this one. This is a blocker actually because the drag&drop feature is broken
(Assignee)

Comment 34

5 years ago
I have a solution without removing any effect hopefully. I gonna upload the patch in one hour.
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
(Assignee)

Comment 35

5 years ago
Created attachment 8345793 [details]
Patch v1

Could you review and check it in your device? thanks a lot
Attachment #8345793 - Flags: review?(ran)

Updated

5 years ago
Component: DOM: CSS Object Model → Gaia::Homescreen
Product: Core → Firefox OS
Comment on attachment 8345793 [details]
Patch v1

Perfect!
Attachment #8345793 - Flags: review?(ran) → review+
(Assignee)

Comment 37

5 years ago
The Travis CI build passed but I don't see the merge button :(
(Assignee)

Comment 38

5 years ago
https://github.com/crdlc/gaia/commit/0294f589b11ad5f53250f8713fb82f59e55c0fcc
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
blocking+ for regression
blocking-b2g: 1.3? → 1.3+
Uplifted 0294f589b11ad5f53250f8713fb82f59e55c0fcc to:
v1.3: fc3062952ba64b22dc19f18f3d8fd9832a1bab49
status-b2g-v1.3: --- → fixed
You need to log in before you can comment on or make changes to this bug.