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

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Tim Nguyen :ntim]

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.

Comment 1

[Paul Rouget [:paul]]

I can reproduce.

Comment 2

[Patrick Brosset <:pbro>]

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.

Comment 3

[Patrick Brosset <:pbro>]

Attachment: bug940500-colorpicker-tooltip-breaks-gradients.patch

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

Comment 4

[Patrick Brosset <:pbro>]

Sorry I somehow messed up the try build syntax. Here's a new try build:
https://tbpl.mozilla.org/?tree=Try&rev=f8c33b8bf2f4

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8340558 [details] [diff] [review]
bug940500-colorpicker-tooltip-breaks-gradients.patch

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

Looks great, r+.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/bac5ac014c63

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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