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

Status: RESOLVED FIXED

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

Description

[Amir Nissim (:amirn)]

See attached video

Comment 1

[Amir Nissim (:amirn)]

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

Comment 2

[Cristian Rodriguez (:crdlc)]

what info do you want? There is a bug :D

Comment 3

[Cristian Rodriguez (:crdlc)]

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

Comment 4

[Cristian Rodriguez (:crdlc)]

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?

Comment 5

[Cristian Rodriguez (:crdlc)]

Thanks Jason, I didn't have idea which component I should set :(

Comment 7

[J Zimbrick]

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

Comment 8

[Jason Smith [:jsmith]]

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.

Comment 9

[Jason Smith [:jsmith]]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=153aebb30387&tochange=e4cd2242cc7d

Comment 10

[Jason Smith [:jsmith]]

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?

Comment 11

[Andrew Overholt [:overholt]]

(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.

Comment 12

[Boris Zbarsky [:bzbarsky]]

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.

Comment 13

[Andrew Overholt [:overholt]]

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.

Comment 14

[Cristian Rodriguez (:crdlc)]

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.

Comment 15

[Boris Zbarsky [:bzbarsky]]

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?

Comment 16

[Cristian Rodriguez (:crdlc)]

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

Comment 17

[Cristian Rodriguez (:crdlc)]

Sorry I forgot to say that it was working fine in Gecko 18.1

Comment 18

[Boris Zbarsky [:bzbarsky]]

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.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that tods NOT show the problem

Comment 20

[Cristian Rodriguez (:crdlc)]

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

Comment 21

[Cristian Rodriguez (:crdlc)]

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

Comment 22

[Evyatar 'Tron' Amitay (everything.me)]

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?

Comment 23

[Cristian Rodriguez (:crdlc)]

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

Comment 24

[Evyatar 'Tron' Amitay (everything.me)]

Hmm well the transition adds to the look and feel, but of course we can give it up if it's so problematic...

Comment 25

[Boris Zbarsky [:bzbarsky]]

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?

Comment 26

[Cristian Rodriguez (:crdlc)]

(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

Comment 27

[Evyatar 'Tron' Amitay (everything.me)]

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.

Comment 28

[Ran Ben Aharon [:ranbena] EverythingMe]

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?

Comment 29

[Boris Zbarsky [:bzbarsky]]

I would still dearly love someone to give me a way to reproduce the Gecko issue.

Comment 30

[Patryk Adamczyk [:patryk] UX]

> 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?

Comment 31

[Ran Ben Aharon [:ranbena] EverythingMe]

If triage allows it, I would rather the Bug stay open till it's fixed with no workaround.

Comment 33

[Cristian Rodriguez (:crdlc)]

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

Comment 34

[Cristian Rodriguez (:crdlc)]

I have a solution without removing any effect hopefully. I gonna upload the patch in one hour.

Comment 35

[Cristian Rodriguez (:crdlc)]

Attachment: Patch v1

Could you review and check it in your device? thanks a lot

Comment 36

[Ran Ben Aharon [:ranbena] EverythingMe]

Comment on attachment 8345793 [details]
Patch v1

Perfect!

Comment 37

[Cristian Rodriguez (:crdlc)]

The Travis CI build passed but I don't see the merge button :(

Comment 38

[Cristian Rodriguez (:crdlc)]

https://github.com/crdlc/gaia/commit/0294f589b11ad5f53250f8713fb82f59e55c0fcc

Comment 39

[Jason Smith [:jsmith]]

blocking+ for regression

Comment 40

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 0294f589b11ad5f53250f8713fb82f59e55c0fcc to:
v1.3: fc3062952ba64b22dc19f18f3d8fd9832a1bab49