Closed Bug 924843 Opened 6 years ago Closed 6 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

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

RESOLVED FIXED
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed

People

(Reporter: amirn, Assigned: crdlc)

References

Details

(Keywords: regression)

Attachments

(2 files)

See attached video
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
Flags: needinfo?(crdlc)
what info do you want? There is a bug :D
Flags: needinfo?(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
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Keywords: regression
Depends on: 910324
Assignee: crdlc → nobody
No longer depends on: 910324
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
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
Component: Builds → General
Thanks Jason, I didn't have idea which component I should set :(
Status: ASSIGNED → NEW
Duplicate of this bug: 944596
Blocks: 1.3-e.me
blocking-b2g: --- → 1.3?
Whiteboard: [systemsfe]
QA Contact: jzimbrick
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
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.
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.
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)
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)
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.
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)
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)
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?
(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.
Duplicate of this bug: 948652
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
I have a solution without removing any effect hopefully. I gonna upload the patch in one hour.
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Attached file Patch v1
Could you review and check it in your device? thanks a lot
Attachment #8345793 - Flags: review?(ran)
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+
The Travis CI build passed but I don't see the merge button :(
https://github.com/crdlc/gaia/commit/0294f589b11ad5f53250f8713fb82f59e55c0fcc
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
blocking+ for regression
blocking-b2g: 1.3? → 1.3+
Uplifted 0294f589b11ad5f53250f8713fb82f59e55c0fcc to:
v1.3: fc3062952ba64b22dc19f18f3d8fd9832a1bab49
You need to log in before you can comment on or make changes to this bug.