Closed
Bug 1453428
Opened 6 years ago
Closed 6 years ago
Shape editor: coordinate units are replaced with px when coordinate value is zero.
Categories
(DevTools :: Inspector: Rules, defect, P1)
DevTools
Inspector: Rules
Tracking
(firefox61 verified, firefox62 verified)
VERIFIED
FIXED
Firefox 61
People
(Reporter: rcaliman, Assigned: rcaliman)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
Steps: - Paste and run this in the URL bar: data:text/html,<style>div {width:300px;height:300px;background:red;clip-path:polygon(0% 0%, 0% 100%, 100% 100%);}</style><div> - Open the Inspector and inspect the div. - Trigger the shapes editor on the "clip-path" property. - Observe the polygon's first coordinate, 0% 0%, has percentages as units. - Using the shape editor, drag the first point. Result: Coordinate unit types changed to pixels replacing percentages. Expected: Values should update, but coordinate unit types should not change. For unit-less zero, it's ok to default to pixels. The same behaviour occurs for other all unit types which are not pixels (%, em, rem, vh, vw). It also happens when editing values of the `shape-outside` CSS property.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•6 years ago
|
||
Latest try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee1b8afbe8ebe46c601a065acfeb95b6e6ec4c71
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8967318 [details] Bug 1453428 - Handle cases for shape coordinates like 0%, 0em, 0rem, etc. without overwriting to pixels. https://reviewboard.mozilla.org/r/236026/#review242482 ::: devtools/server/actors/highlighters/shapes.js:2515 (Diff revision 1) > + * > + * @param {String} unit > + * One of: %, em, rem, vw, vh > + * @param {Number} size > + * Size to which percentage values are relative to. > + * Remove this new line between the @param and @return
Attachment #8967318 -
Flags: review?(gl) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Updated•6 years ago
|
Priority: -- → P1
Comment 5•6 years ago
|
||
Again the r+ in MozReview has been lost. Razvan, Gabriel, please track down why this always happens for you and how you can prevent or fix that. Thank you. r+ on the left side in MozReview is needed to land the patch.
Flags: needinfo?(gl)
Updated•6 years ago
|
Flags: needinfo?(gl)
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3f95bbb60e43 Handle cases for shape coordinates like 0%, 0em, 0rem, etc. without overwriting to pixels. r=gl
Keywords: checkin-needed
Comment 7•6 years ago
|
||
Backed out changeset 3f95bbb60e43 (bug 1453428) for mochitest devtools failures on browser_inspector_highlighter-cssshape_iframe_01.js Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/bdbed4e49efb5756f97cb2a913eb96f62495429a Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=3f95bbb60e4399e9b7533aec0f42e52539f8b235&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=retry&filter-resultStatus=pending&filter-resultStatus=running&filter-resultStatus=superseded&selectedJob=174126579 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=174126579&repo=mozilla-inbound&lineNumber=1857 08:52:25 INFO - TEST-START | devtools/client/inspector/test/browser_inspector_highlighter-cssshape_iframe_01.js 08:52:27 INFO - TEST-INFO | started process screencapture 08:52:27 INFO - TEST-INFO | screencapture: exit 0 08:52:27 INFO - Buffered messages logged at 08:52:25 08:52:27 INFO - Entering test bound 08:52:27 INFO - Adding a new tab with URL: http://example.com/browser/devtools/client/inspector/test/doc_inspector_highlighter_cssshapes_iframe.html 08:52:27 INFO - Buffered messages logged at 08:52:26 08:52:27 INFO - Tab added and finished loading 08:52:27 INFO - Opening the inspector 08:52:27 INFO - Opening the toolbox 08:52:27 INFO - Toolbox opened and focused 08:52:27 INFO - Turn on shapes highlighter for #polygon 08:52:27 INFO - Selecting the node for '[Front for domnode/server1.conn36.child1/domnode43]' 08:52:27 INFO - Toggle shapes highlighter on for clip-path on #polygon 08:52:27 INFO - Buffered messages logged at 08:52:27 08:52:27 INFO - Moving polygon point visible in iframe 08:52:27 INFO - Buffered messages finished 08:52:27 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/test/browser_inspector_highlighter-cssshape_iframe_01.js | Point moved to 10px 1.25% - 08:52:27 INFO - Stack trace: 08:52:27 INFO - chrome://mochitests/content/browser/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_iframe_01.js:testPolygonIframeMovePoint:47 08:52:27 INFO - Moving polygon point not visible in iframe 08:52:27 INFO - TEST-PASS | devtools/client/inspector/test/browser_inspector_highlighter-cssshape_iframe_01.js | Point moved to 110px 51.25% - 08:52:27 INFO - Turn off shapes highlighter for #polygon 08:52:27 INFO - Toggle shapes highlighter off for clip-path on #polygon 08:52:27 INFO - Leaving test bound 08:52:27 INFO - Removing tab.
Flags: needinfo?(rcaliman)
Assignee | ||
Comment 8•6 years ago
|
||
Forwarding r+ from previous patch. Strange if that mochitest failed like that. The patch introduces a fix for that exact issue. I rebased my changes, ran mochitests (ok), and pushed to try (green). Attaching patch for check-in again. Latest try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d24407090807fed00b5de34ff0ac83ce3848974e
Attachment #8967318 -
Attachment is obsolete: true
Flags: needinfo?(rcaliman)
Attachment #8968632 -
Flags: review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b4a808050676 Handle cases for shape coordinates like 0%, 0em, 0rem, etc. without overwriting to pixels. r=gl
Keywords: checkin-needed
Comment 10•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b4a808050676
Updated•6 years ago
|
QA Whiteboard: [good first verify]
Comment 11•6 years ago
|
||
I have reproduced this bug with Nightly 61.0a1 (2018-04-11) on Windows 10 , 64 Bit ! This bug's fix is Verified with latest Beta ! Build ID 20180517141400 User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0 [bugday-20180516]
Comment 12•6 years ago
|
||
Hi everyone, i have retested this issue on Windows 10 , Windows 7 , Mac OSx and Linux using the latest version of nightly 62.0a1 (2018-05-25) and i can Confirm it as Fixed. I will mark it accordingly.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•