1243409 - Dragging element leaves traces

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Oriol Brufau [:Oriol]]

Attachment: screenshot

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160126030244

Steps to reproduce:

1. Go to http://live.yworks.com/yfiles-for-html/1.3/demos/complete/demo.yfiles.graph.bpmneditor/index.html
2. Mousedown some blue area of the diagram
3. Move the mouse to drag the diagram



Actual results:

The diagram leaves traces.


Expected results:

It shouldn't leave traces.

Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c525c516&tochange=c99e8abb

So the cause is bug 1231538 or bug 1221842. I suspect it's the later.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Always compare geometries

Previously we skipped comparing geometry objects for item types that created layers as they would be handled by the layer tree comparison and there was no need to invalidate twice.

With the caching however it means that we think we can reuse the old geometry object even though it might be out of date.

When we come to an invalidation that does require the geometry object (removing the item from the layer), the bounds are out of date and we invalidate incorrectly.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8712957 [details] [diff] [review]
Always compare geometries

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

Testcase?

Comment 5

[Oriol Brufau [:Oriol]]

Attachment: testcase

Comment 6

[Benjamin Kerensa [:bkerensa]]

Matt is 46 impacted by this bug?

Comment 7

[Benjamin Kerensa [:bkerensa]]

[Tracking Requested - why for this release]:

Comment 8

[Loic]

Only FF47 is affected.

Comment 9

[Oriol Brufau [:Oriol]]

The patch has review+ and fixes my testcase. Why has it not been implemented already?

Comment 10

[YF (Yang)]

(In reply to Oriol from comment #9)
> The patch has review+ and fixes my testcase. Why has it not been implemented
> already?

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to Oriol from comment #9)
> The patch has review+ and fixes my testcase. Why has it not been implemented already?

Right, Matt hasn't landed the patch yet -- my guess is that's because he's working on making an automated testcase, in response to comment 4 (which our infrastructure can use to verify whether the bug is fixed, and make sure it doesn't break again). (and he's got other stuff on his plate, too)

This bug is already tagged as needinfo=matt.woodrow, so it's on his queue of things to do. Hopefully he'll be able to get to it soon! :)

(In reply to YF (Yang) from comment #10)
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree

(This MDN link mostly describes the "checkin-needed" keyword, which isn't appropriate here, until/unless Matt says his patch is ready to land.)

Comment 12

[Daniel Holbert [:dholbert]]

(Your testcase in comment 5 may provide most of what's needed for an automated testcase; I'm not sure. Anyway, Matt can comment more.)

Comment 13

[Matt Woodrow (:mattwoodrow)]

Attachment: Always compare geometries

The previous version was failing existing automated tests and couldn't be landed.

This new version passes all existing tests, and adds a new one (using the provided testcase).

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea6fa3f97f98

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ea6fa3f97f98