Closed Bug 1160720 Opened 5 years ago Closed 5 years ago

The color picker starts to follow the pointer if the pointer is pressed down and then released outside the tooltip frame (in the markup view)

Categories

(DevTools :: Inspector, defect)

Unspecified
Linux
defect
Not set

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: sjakthol, Assigned: sjakthol)

Details

Attachments

(1 file, 1 obsolete file)

Steps to reproduce:

1. Go to data:text/html;charset=utf-8,<div style="color: red">red
2. Inspect the red text
3. Click on the color swatch in the rule-view
4. Press mouse down to select a color and drag it
5. Drag the pointer outside the tooltip frame and release the mouse button
6. Move the pointer back inside the color picker and move it around

What happens: The color changes constantly as the picker keeps following the mouse.

What should happen: The picker should not follow the pointer if the mouse button is not pressed down.

The spectrum [1] should probably check the buttons attribute of the mousemove [2] event to check if a button is still pressed as it can't receive mouseup events that happen outside the frame to abort the movement.

[1] https://dxr.mozilla.org/mozilla-central/source/browser/devtools/shared/widgets/Spectrum.js#164
[2] https://w3c.github.io/uievents/#widl-MouseEvent-buttons
Here's a fix for this. It aborts the drag if there's no buttons pressed down on mousemove.

The test in this patch is quite complex. I tried to write simpler one in shared/test/browser_spectrum.js but for some reason I wasn't able to get the widget to hear events sent via EventUtils there.
Assignee: nobody → sjakthol
Status: NEW → ASSIGNED
Attachment #8600973 - Flags: review?(pbrosset)
Comment on attachment 8600973 [details] [diff] [review]
bug-1160720-color-picker-tooltip.patch

Thanks Sami for working on this. Unfortunately I'll be off the rest of this week, and Brian should be a better reviewer than me for this since he's the author of the Spectrum widget.
Attachment #8600973 - Flags: review?(pbrosset) → review?(bgrinstead)
Hi Sami, thanks for the patch.  I'm not able to reproduce the problem (I've checked on osx, windows and linux while mousing up outside of the tooltip frame, and even outside of the entire browser window.  The fix makes sense, but I'd like to narrow down exactly the scenario this happens in.
Flags: needinfo?(sjakthol)
Note: the test case does fails for me without the fix applied but I still can't figure out how to reproduce manually
Interesting. I can't reproduce this on Windows and now that you pointed it out I can't always reproduce it on Ubuntu 14.04 either.

For example, if I release the button over the page content, this doesn't seem to happen. On Ubuntu 14.04 the best way I've figured out so far is to release the mouse button on top of the markup view.

That seems to reliably reproduce the issue but it sure seems odd that it only happens on certain platforms and when the mouse is released at specific points. Or maybe this happens to me due to some setting I've toggled or an application I've installed. I'll check if I can reproduce this with stock Ubuntu 14.04 installation later.
Flags: needinfo?(sjakthol)
(In reply to Sami Jaktholm from comment #6)
> Interesting. I can't reproduce this on Windows and now that you pointed it
> out I can't always reproduce it on Ubuntu 14.04 either.
> 
> For example, if I release the button over the page content, this doesn't
> seem to happen. On Ubuntu 14.04 the best way I've figured out so far is to
> release the mouse button on top of the markup view.

OK, thanks for the info.  I was able to reproduce by mousing up somewhere in the markup view on Ubuntu.
Summary: The color picker starts to follow the pointer if the pointer is pressed down and then released outside the tooltip frame → The color picker starts to follow the pointer if the pointer is pressed down and then released outside the tooltip frame (in the markup view)
OS: Unspecified → Linux
Comment on attachment 8600973 [details] [diff] [review]
bug-1160720-color-picker-tooltip.patch

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

Looks good.  Please update the commit message with r=bgrins
Attachment #8600973 - Flags: review?(bgrinstead) → review+
Rebased patch with correct reviewer and updated commit message.
Attachment #8602172 - Flags: review+
Attachment #8600973 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/019c10ffbe16
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.