Closed Bug 940500 Opened 6 years ago Closed 6 years ago

Editing colors in a gradient using the color picker tooltip breaks the gradient

Categories

(DevTools :: Inspector, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 28

People

(Reporter: ntim, Assigned: pbro)

References

Details

Attachments

(1 file)

When you edit a color in a gradient with the color picker tooltip, it will remove the comma following the color, which will break the gradient.

Also, it doesn't respect the color unit setting but I'll file another bug for that.
Depends on: 889638
Blocks: 889638
No longer depends on: 889638
I can reproduce.
Status: UNCONFIRMED → NEW
Ever confirmed: true
The problem is that so far, the colorpicker looks for the nextSibling element of the color swatch that owns it.
Although this is fine in simple cases, as soon as you have more complex CSS values like gradients, the nextSibling isn't going to be only the color, as illustrated below:

<span class="swatch">O</span> <textNode>#345678 10%, </textNode>

Indeed, the nextSibling is a textNode that not only contains the color value but also the color stop and comma character.

Let's improve the output-parser by making it enclose the color in a span just like it does for the swatch.
This way, it'll be very easy to get this element and we'll be sure then that it only contains the color value.
Assignee: nobody → pbrosset
Mike, this review is for you since you know the output-parser best.

This is a simple change as explained in my comment above: adding a <span> element around the color value so that the color picker can find it easily and edit its text content.

Ongoing try build https://tbpl.mozilla.org/?tree=Try&rev=9f8996a31d8b
Attachment #8340558 - Flags: review?(mratcliffe)
Sorry I somehow messed up the try build syntax. Here's a new try build:
https://tbpl.mozilla.org/?tree=Try&rev=f8c33b8bf2f4
Comment on attachment 8340558 [details] [diff] [review]
bug940500-colorpicker-tooltip-breaks-gradients.patch

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

Looks great, r+.
Attachment #8340558 - Flags: review?(mratcliffe) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/bac5ac014c63
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/bac5ac014c63
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.