Closed Bug 1243409 Opened 7 years ago Closed 7 years ago

Dragging element leaves traces

Categories

(Core :: Layout, defect)

47 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- unaffected
firefox46 --- unaffected
firefox47 + fixed

People

(Reporter: Oriol, Assigned: mattwoodrow)

References

()

Details

(Keywords: regression, testcase)

Attachments

(3 files, 1 obsolete file)

Attached image 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.
Flags: needinfo?(matt.woodrow)
Attached patch Always compare geometries (obsolete) — Splinter Review
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.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Attachment #8712957 - Flags: review?(roc)
Comment on attachment 8712957 [details] [diff] [review]
Always compare geometries

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

Testcase?
Attachment #8712957 - Flags: review?(roc) → review+
Version: unspecified → 47 Branch
Attached file testcase
Matt is 46 impacted by this bug?
Flags: needinfo?(matt.woodrow)
[Tracking Requested - why for this release]:
Only FF47 is affected.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
The patch has review+ and fixes my testcase. Why has it not been implemented already?
(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
(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.)
(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.)
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).
Attachment #8712957 - Attachment is obsolete: true
Flags: needinfo?(matt.woodrow)
Attachment #8717286 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/ea6fa3f97f98
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.