Units cycling with shift+click should persists and change the value in Style editor

VERIFIED FIXED in Firefox 48

Status

()

Firefox
Developer Tools: CSS Rules Inspector
P2
normal
VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

(Depends on: 1 bug)

48 Branch
Firefox 48
Points:
---

Firefox Tracking Flags

(firefox48 verified)

Details

(Whiteboard: [btpp-fix-later])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Created attachment 8734496 [details]
angle.html

For color and angle values, we can cycle through the units ( rgb -> hex -> hsl -> name , deg -> rad -> grad -> turn ) with shift + click.
Those unit changes don't persist though.

STR : 
1. Open the attached file
2. Open inspector
3. Select the <body> node
4. shift + click on the `rgba(183,222,237,1)` color swatch to the hex value (`#b7deed`)
5. select the <html> node
6. select the <body> node


Expected result : 
The first color of the gradient should be `#b7deed`.


Actual result : 
The first color of the gradient is `rgba(183,222,237,1)`, the original value.
(Assignee)

Updated

2 years ago
Has STR: --- → yes
Component: Developer Tools → Developer Tools: CSS Rules Inspector
Version: 47 Branch → 48 Branch
(Assignee)

Updated

2 years ago
Assignee: nobody → chevobbe.nicolas
Triaging (filter on CLIMBING SHOES).
Priority: -- → P2
Whiteboard: [btpp-fix-later]
(Assignee)

Comment 2

2 years ago
So I thought a little more about this one and there is something that should be taken care of. In the option panel, you can chose how colours should be displayed. It can be "authored", i.e. like it is declared by the user in the rule, or it can be one of hex, hsl, rgba, name.

Maybe we could only persist value through shift+click if the pref is set to "authored". But it can be confusing to both persist and not, only depending of the state of a preference.

What do you think about it ?
Flags: needinfo?(ntim.bugs)
Flags: needinfo?(hholmes)
I think we should always persist, whether it was authored or not. I think that the changing would be more confusing than anything else, like you've already mentioned.
Flags: needinfo?(hholmes)

Comment 4

2 years ago
We should persist in any case. Editing the unit manually without shift click does persist, and we should do the same with shift click
Flags: needinfo?(ntim.bugs)
(Assignee)

Comment 5

2 years ago
Created attachment 8736013 [details]
MozReview Request: Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker

Add an 'unit-change' event fired when shift+click on color and angle swatches.
Add a listener on this event in text-property-editor.js to call the same function
that's called when tooltip edit is commited to persist the new unit.
Edit some tests to adapt to this new behaviour and create some tests to make sure
the value obtained via shift+click are actually persisted.

Review commit: https://reviewboard.mozilla.org/r/43039/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43039/
Attachment #8736013 - Flags: review?(mratcliffe)

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 6

2 years ago
Comment on attachment 8736013 [details]
MozReview Request: Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43039/diff/1-2/
Comment on attachment 8736013 [details]
MozReview Request: Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker

https://reviewboard.mozilla.org/r/43039/#review39733

Looks spot on... r+ assuming a green try.

::: devtools/client/inspector/rules/test/browser_rules_cycle-color.js:18
(Diff revision 2)
>      }
> +    span {
> +      color: blue;
> +    }
>    </style>
> -  Test cycling color types in the rule view!
> +  <body><span>Test</span>cycling color types in the rule view!</body>

Nit: Add a space after the closing span.
Attachment #8736013 - Flags: review?(mratcliffe) → review+
(Assignee)

Comment 8

2 years ago
Comment on attachment 8736013 [details]
MozReview Request: Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43039/diff/2-3/
(Assignee)

Comment 9

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ddcc57f9baf

The TRY run is over and i think everything is good

Errors known : 

dt8 on Windows 7 debug : browser_webconsole_bug_632817.js https://bugzilla.mozilla.org/show_bug.cgi?id=1244707

dt3 on Linux debug| dt2 on Linux 64 debug : No bug filed, but appears in another TRY run without my patch ( https://treeherder.mozilla.org/#/jobs?repo=try&revision=7db58284a44ef0cbd4518ae34419a66ed1f82b35&selectedJob=18769152 )
Keywords: checkin-needed

Comment 10

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/6d91363a7eac
Keywords: checkin-needed

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6d91363a7eac
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48

Comment 12

a year ago
I have reproduced this bug with nightly 48.0a1 (2016-03-24) on Windows 10, 64 bit!

The Bug's fix is now verified on Latest Beta 48.0b6

Build ID 	20160706215822
User Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

[bugday-20160713]

Updated

a year ago
Status: RESOLVED → VERIFIED
status-firefox48: fixed → verified
Depends on: 1284852
You need to log in before you can comment on or make changes to this bug.