User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0 Build ID: 20181008100121 STR: 1. Launch Firefox and navigate to a random page (e.g. nytimes.com) 2. Enable the element inspector. 3. Choose a Flex Container or Item and enable the highlighter. 4. Navigate to the Flexbox Inspector section and select the color picker. 5. Try changing the overlay color. 6. Observe the overlay color on the webpage while the user changes colors in the color picker. Actual result: The color selected by the user is not reflected on the page. *Workaround: The change can be observed if the flex overlay is disabled and then re-enabled. I have attached a short video for better understanding. Expected result: The user can choose a new color for the highlighter overlay and the color is properly reflected in the webpage.

Comment on attachment 9016021 [details] [diff] [review] 1497312.patch [1.0] Review of attachment 9016021 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/inspector/flexbox/test/browser_flexbox_highlighter_color_picker_on_ESC.js @@ +2,5 @@ > + http://creativecommons.org/publicdomain/zero/1.0/ */ > + > +"use strict"; > + > +// Test that the flexbox highlighter color change in the color picker is reverted when Thank you for adding this test (and the other one). Only comment I have is that they claim to be testing the highlighter color, but really they test the color of the swatch in the panel. Can we also somehow test the color sent to the highlighter? I know it uses canvas to draw, so that might be hard. But maybe there's a way we can check the call to the highlighter (maybe stubbing the highlighter's show/hide methods so it doesn't actually appear, but we get to see what options it is called with).

I don't know if we could do any better here.

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ffb2909f660d The flexbox highlighter color should change on input from the color swatch. r=pbro

Backed out changeset ffb2909f660d (Bug 1497312) for browser_flexbox_highlighter_color_picker_on_RETURN.js failures. Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&selectedJob=205603026&revision=ffb2909f660d4ea2f9961e5d1d4964a2c98e51a3 Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/20cc444ecb1222fc280c08e658fe1b22de00d1a7 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=205603026&repo=mozilla-inbound&lineNumber=1635 [task 2018-10-15T19:46:14.436Z] 19:46:14 INFO - TEST-PASS | devtools/client/inspector/flexbox/test/browser_flexbox_highlighter_color_picker_on_RETURN.js | The flexbox color state is correct. - [task 2018-10-15T19:46:14.437Z] 19:46:14 INFO - Toggling ON the flexbox highlighter. [task 2018-10-15T19:46:14.438Z] 19:46:14 INFO - Waiting for state predicate "state => state.flexbox.highlighted" [task 2018-10-15T19:46:14.438Z] 19:46:14 INFO - Found state predicate "state => state.flexbox.highlighted" [task 2018-10-15T19:46:14.439Z] 19:46:14 INFO - Opening the color picker by clicking on the color swatch. [task 2018-10-15T19:46:14.440Z] 19:46:14 INFO - Buffered messages logged at 19:46:13 [task 2018-10-15T19:46:14.440Z] 19:46:14 INFO - Getting the spectrum colorpicker object [task 2018-10-15T19:46:14.441Z] 19:46:14 INFO - Setting the new color [task 2018-10-15T19:46:14.442Z] 19:46:14 INFO - Applying the change [task 2018-10-15T19:46:14.442Z] 19:46:14 INFO - Checking the flexbox highlighter display settings. [task 2018-10-15T19:46:14.443Z] 19:46:14 INFO - TEST-PASS | devtools/client/inspector/flexbox/test/browser_flexbox_highlighter_color_picker_on_RETURN.js | Flexbox highlighter color is correct. - [task 2018-10-15T19:46:14.443Z] 19:46:14 INFO - Buffered messages finished [task 2018-10-15T19:46:14.446Z] 19:46:14 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/flexbox/test/browser_flexbox_highlighter_color_picker_on_RETURN.js | The color swatch's background was updated. - Got rgb(148, 0, 255), expected rgba(0, 255, 0, 0.5) [task 2018-10-15T19:46:14.448Z] 19:46:14 INFO - Stack trace: [task 2018-10-15T19:46:14.449Z] 19:46:14 INFO - chrome://mochikit/content/browser-test.js:test_is:1295 [task 2018-10-15T19:46:14.450Z] 19:46:14 INFO - chrome://mochitests/content/browser/devtools/client/inspector/flexbox/test/browser_flexbox_highlighter_color_picker_on_RETURN.js:null:51 [task 2018-10-15T19:46:14.452Z] 19:46:14 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1093 [task 2018-10-15T19:46:14.453Z] 19:46:14 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1084 [task 2018-10-15T19:46:14.454Z] 19:46:14 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:986 [task 2018-10-15T19:46:14.454Z] 19:46:14 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795 [task 2018-10-15T19:46:14.455Z] 19:46:14 INFO - Pressing RETURN to commit the color change. [task 2018-10-15T19:46:14.456Z] 19:46:14 INFO - Waiting for state predicate "state => [task 2018-10-15T19:46:14.457Z] 19:46:14 INFO - state.flexbox.color === "#00FF0080"" [task 2018-10-15T19:46:14.458Z] 19:46:14 INFO - GECKO(1073) | console.log: "[DISPATCH] action type:" "UPDATE_FLEXBOX_COLOR" [task 2018-10-15T19:46:14.459Z] 19:46:14 INFO - GECKO(1073) | console.log: "[DISPATCH] action type:" "UPDATE_FLEXBOX_COLOR" [task 2018-10-15T19:46:14.467Z] 19:46:14 INFO - GECKO(1073) | --DOMWINDOW == 40 (0x7f6ea4da8400) [pid = 1073] [serial = 30] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:14.468Z] 19:46:14 INFO - GECKO(1073) | --DOMWINDOW == 39 (0x7f6ec551c800) [pid = 1073] [serial = 28] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:14.469Z] 19:46:14 INFO - GECKO(1073) | --DOMWINDOW == 38 (0x7f6eafc1e400) [pid = 1073] [serial = 10] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:14.470Z] 19:46:14 INFO - GECKO(1073) | --DOMWINDOW == 37 (0x7f6eafc1d400) [pid = 1073] [serial = 8] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:14.473Z] 19:46:14 INFO - GECKO(1073) | --DOMWINDOW == 36 (0x7f6ebf15d000) [pid = 1073] [serial = 2] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:14.474Z] 19:46:14 INFO - GECKO(1073) | --DOMWINDOW == 35 (0x7f6ed811dc00) [pid = 1073] [serial = 14] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:14.475Z] 19:46:14 INFO - GECKO(1073) | --DOMWINDOW == 34 (0x7f6ec5526400) [pid = 1073] [serial = 18] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:14.476Z] 19:46:14 INFO - GECKO(1073) | --DOMWINDOW == 33 (0x7f6ec5522800) [pid = 1073] [serial = 17] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:15.105Z] 19:46:15 INFO - GECKO(1073) | --DOCSHELL 0x7f6ea817d000 == 11 [pid = 1073] [id = {5abed056-4f89-4ce7-a29a-2113346b7cf2}] [task 2018-10-15T19:46:17.063Z] 19:46:17 INFO - GECKO(1073) | --DOMWINDOW == 2 (0x7fed767b4000) [pid = 1242] [serial = 2] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:19.626Z] 19:46:19 INFO - GECKO(1073) | --DOMWINDOW == 1 (0x7fdc29d56800) [pid = 1213] [serial = 1] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:19.662Z] 19:46:19 INFO - GECKO(1073) | --DOMWINDOW == 1 (0x7fed8d654600) [pid = 1242] [serial = 1] [outer = (nil)] [url = http://example.com/browser/devtools/client/inspector/flexbox/test/doc_flexbox_simple.html] [task 2018-10-15T19:46:20.405Z] 19:46:20 INFO - GECKO(1073) | --DOMWINDOW == 2 (0x7fc4aee4e800) [pid = 1289] [serial = 2] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:22.197Z] 19:46:22 INFO - GECKO(1073) | --DOMWINDOW == 32 (0x7f6eada34000) [pid = 1073] [serial = 22] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:22.198Z] 19:46:22 INFO - GECKO(1073) | --DOMWINDOW == 31 (0x7f6eb1edcc00) [pid = 1073] [serial = 41] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:22.199Z] 19:46:22 INFO - GECKO(1073) | --DOMWINDOW == 30 (0x7f6eafc5ee00) [pid = 1073] [serial = 25] [outer = (nil)] [url = chrome://devtools/content/inspector/index.xhtml] [task 2018-10-15T19:46:22.201Z] 19:46:22 INFO - GECKO(1073) | --DOMWINDOW == 29 (0x7f6ea4d59800) [pid = 1073] [serial = 31] [outer = (nil)] [url = chrome://devtools/content/inspector/markup/markup.xhtml] [task 2018-10-15T19:46:23.614Z] 19:46:23 INFO - GECKO(1073) | --DOMWINDOW == 0 (0x7fdc13802400) [pid = 1213] [serial = 2] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:23.733Z] 19:46:23 INFO - GECKO(1073) | --DOMWINDOW == 0 (0x7fed75dcc400) [pid = 1242] [serial = 3] [outer = (nil)] [url = http://example.com/browser/devtools/client/inspector/flexbox/test/doc_flexbox_simple.html] [task 2018-10-15T19:46:33.063Z] 19:46:33 INFO - GECKO(1073) | --DOMWINDOW == 28 (0x7f6ea45a5000) [pid = 1073] [serial = 33] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:33.064Z] 19:46:33 INFO - GECKO(1073) | --DOMWINDOW == 27 (0x7f6eac294000) [pid = 1073] [serial = 26] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:33.065Z] 19:46:33 INFO - GECKO(1073) | --DOMWINDOW == 26 (0x7f6eada2ec00) [pid = 1073] [serial = 24] [outer = (nil)] [url = about:devtools-toolbox] [task 2018-10-15T19:46:33.067Z] 19:46:33 INFO - GECKO(1073) | --DOMWINDOW == 25 (0x7f6eac29f800) [pid = 1073] [serial = 42] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:46:33.068Z] 19:46:33 INFO - GECKO(1073) | --DOMWINDOW == 24 (0x7f6ea4db1000) [pid = 1073] [serial = 32] [outer = (nil)] [url = chrome://devtools/content/inspector/markup/markup.xhtml] [task 2018-10-15T19:46:33.068Z] 19:46:33 INFO - GECKO(1073) | --DOMWINDOW == 23 (0x7f6eada28400) [pid = 1073] [serial = 23] [outer = (nil)] [url = about:blank] [task 2018-10-15T19:47:34.591Z] 19:47:34 INFO - Not taking screenshot here: see the one that was previously logged [task 2018-10-15T19:47:34.593Z] 19:47:34 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/flexbox/test/browser_flexbox_highlighter_color_picker_on_RETURN.js | Test timed out - [task 2018-10-15T19:47:34.933Z] 19:47:34 INFO - GECKO(1073) | ++DOMWINDOW == 24 (0x7f6eaa1b1c00) [pid = 1073] [serial = 45] [outer = 0x7f6eada39800] [task 2018-10-15T19:47:35.765Z] 19:47:35 INFO - Removing tab. [task 2018-10-15T19:47:35.769Z] 19:47:35 INFO - Waiting for event: 'TabClose' on [object XULElement]. [task 2018-10-15T19:47:35.814Z] 19:47:35 INFO - Got event: 'TabClose' on [object XULElement]. [task 2018-10-15T19:47:35.850Z] 19:47:35 INFO - Tab removed and finished closing

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/458bf1864db0 The flexbox highlighter color should change on input from the color swatch. r=pbro

Verified using the latest Nightly build: 20181017101626 on Windows 10, Ubuntu 18.04 and Mac OSX, and part of the issue is fixed. The overlay color does change on user input and the color change is reflected on the color picker "icon". However, there are some small issues that are still persistent, namely the actual color of the color picker "icon" and the persistence of the selected color. 1. The color of the picker changes just for a quick second when the user selects a new color using the dropper, as it defaults back to the original one. 2. If the user navigates to the sizing info display of one flex item that is part of the selected container and then back, the overlay does maintain its color, but the color on the color picker "icon" defaults to the original one. The same issue is observed if the selected flex item is actually an inner container. The color of the color picker displayed next to the inner flex container is the default one, instead of the color that the user previously selected and is properly reflected by the overlay. Furthermore, if the toggle is enabled in order to view just the overlay for the inner container, the color of the overlay is changed to the default one. This issue is somewhat the same as if the user chooses to select a new flex element. In this case, the color picker defaults back to the original color. As far as I remember, Victoria mentioned that the color selection should be persistent.

I know it's a low priority issue, and probably we can treat it/edit as such (P4, P5), but I am going to reopen this ticket, and maybe, whenever there is availability, this can be addressed: 1. The color picker icon is reset to the default color, although the overlay color reflects the color previously modified by the user, in a number of scenarios like: page refresh; user enabling the sizing info display of a flex child and then going back to the properties display; another element from the 1st panel is selected and then the user navigates back to the flex-element; inspector is resized 2. The color selected using the dropper is working now for the overlay but is still not reflected on the same color picker icon 3. If the color is selected using the big square, and the selector is moved anywhere on the left or bottom edge, then closed and reopened, the vertical color selector defaults to the top position, making the color of the big square selector, red. I'm going to add a short video of this small issues, for a better understanding.

I've edited the title of the issue in order to better reflect the current behavior.

The color picker does not work in iFrame scenarios. The color change does not apply to the overlay and the color of the color picker defaults back to the original as soon as the user is done selecting a different color. I have attached a short video of the issue.

I was incorrectly renaming hostname to hostName at some point for the constant name, and that also changed the property we were fetching from hostname to hostName. The difference is that hostName returns only "https://" whereas hostname returns "https://bugzilla.mozilla.org/attachment.cgi?bugid=1497312&action=enter".

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/864f1996cb75 Use hostname instead of hostName to get the correct the hostname. r=mtigley

Verified using latest nightly build: 20181112100105 on all OS-es and the issue does not seem to be fixed. The color of the picker defaults to the original one once the user navigates to the sizing display of a flex item and then back to the initial flex container layout and the color of the big square pallet changes to red, as the vertical picker defaults to the top position if the user selects a color from the right edge or bottom edge of the big square. Furthermore, if the color is changed on nested containers, this will impact the mini-map, and using the eye-dropper to change the color does not work. I attached a short video of all this. Based on the above, I'm going to re-open this issue.

Seems like I missed one final step in my last patch which was to also store the new color in this._overlayColor which is actually the cached overlay color value that we show.

Comment on attachment 9025439 [details] [diff] [review] 1497312.patch Review of attachment 9025439 [details] [diff] [review]: ----------------------------------------------------------------- Verified that the mini-map and color swatch no longer default back to the original color when using the eye dropper tool after changing colors on a nested container. As for the issue with the color picker defaulting back to red when the very bottom corners are selected, I think it has to do with the color picker itself than the flexbox inspector. I see this happening with the Grid Inspector too. So I don't think this issue should be addressed in this patch, but in another one instead.

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9864c98ab553 Store the overlay color when the overlay color is changed from the color picker. r=mtigley

(In reply to Micah Tigley [:mtigley] from comment #19) > As for the issue with the color picker defaulting back to red when the very > bottom corners are selected, I think it has to do with the color picker > itself than the flexbox inspector. I see this happening with the Grid > Inspector too. So I don't think this issue should be addressed in this > patch, but in another one instead. I added a new report based on the comment above - https://bugzilla.mozilla.org/show_bug.cgi?id=1510038 On the same topic, I've also added a new report that although it is not generated by this feature, it does impact it as it reproduces on every color picker - https://bugzilla.mozilla.org/show_bug.cgi?id=1510036 The persistence issue is still not fully fixed, therefore I added a new report here: https://bugzilla.mozilla.org/show_bug.cgi?id=1510023.