Closed
Bug 1259559
Opened 8 years ago
Closed 8 years ago
Units cycling with shift+click should persists and change the value in Style editor
Categories
(DevTools :: Inspector: Rules, defect, P2)
Tracking
(firefox48 verified)
VERIFIED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | verified |
People
(Reporter: nchevobbe, Assigned: nchevobbe)
References
Details
(Whiteboard: [btpp-fix-later])
Attachments
(2 files)
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•8 years ago
|
Has STR: --- → yes
Component: Developer Tools → Developer Tools: CSS Rules Inspector
Version: 47 Branch → 48 Branch
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → chevobbe.nicolas
Comment 1•8 years ago
|
||
Triaging (filter on CLIMBING SHOES).
Priority: -- → P2
Whiteboard: [btpp-fix-later]
Assignee | ||
Comment 2•8 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)
Comment 3•8 years ago
|
||
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•8 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•8 years ago
|
||
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•8 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•8 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•8 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•8 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•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6d91363a7eac
Keywords: checkin-needed
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6d91363a7eac
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment 12•8 years 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•8 years ago
|
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•