Closed Bug 1169667 Opened 6 years ago Closed 6 years ago
Tools: when toggling the "Highlight painted area" tool, the icon color is wrong
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:41.0) Gecko/20100101 Firefox/41.0 Build ID: 20150528030206 Steps to reproduce: 1. Open DevTools and enable the "Highlight painted area" tool. 2. Click on the "Highlight painted area" toolbar button. 3. The function is turned on and the icon gets blue. 4. Click again to turn highlighting off. Actual results: - highligthing is turned off, that's OK - but the icon remains blue, as if the tool was still active - the <toolbarbutton> still has attribute checked="true" Expected results: - icon is toggled back to disabled state, has black color - the <toolbarbutton> is not in "checked" state
I can reproduce this in FDE and Nightly, but not Beta 39.0 or Release 38.0.1. Disabling e10s in Nightly and restarting made no difference. Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0 20150529030205 Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:40.0) Gecko/20100101 Firefox/40.0 20150529004007
Component: Developer Tools: Framework → Developer Tools: Graphic Commandline and Toolbar
2015-03-29: works properly. https://hg.mozilla.org/mozilla-central/rev/385840329d91 2015-03-30 → 2015-04-23: with e10s, clicking the button does nothing besides flooding the Browser Console with errors; without e10s, it works properly. https://hg.mozilla.org/mozilla-central/rev/6082a98d3861 https://hg.mozilla.org/mozilla-central/rev/0b202671c9e2 2015-04-24: the button now works with e10s again, but exhibits the bug described in this report, whether e10s is enabled or not. https://hg.mozilla.org/mozilla-central/rev/22a157f7feb7
I've got the same problem on `41.0a1 (2015-06-02)` without e10s enabled.
Working on this, please assign to me.
(In reply to Jarda Snajdr [:jsnajdr] from comment #4) > Working on this, please assign to me. Thank you.
Assignee: nobody → jsnajdr
Here is a patch. The bug was caused by incorrect handling of paintflashing_server command output - an Output object was treated as boolean. Added a test that check the correct state.isChecked() status after every change.
Attachment #8616613 - Flags: review?(jwalker)
6 years ago
Attachment #8616613 - Flags: review?(jwalker) → review+
Reminder to push to TRY when the tree re-opens.
Pending try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=792d8f928cb7
The test browser/devtools/shared/test/browser_telemetry_button_paintflashing.js fails after my patch. The DEVTOOLS_PAINTFLASHING_TIME_ACTIVE_SECONDS is an array that should have 2 entries, each containing for how long the tool has been active. Because the toggle button has been clicked 4 times. However, it contains only one record, because the check happens immediately after calling the last click(), without waiting for the async action to be finished. The ACTIVE_SECONDS record is added only after the tool is closed and the active time can be computed. Before my patch, the telemetry results didn't contain any ACTIVE_SECONDS record at all (which the test fails to notice), so no test on it was really performed. That was caused by the bug being fixed - the paintflashing tool always reported its state as "opened". The computation of active time never starts. To fix: - instead of calling setTimeout, we should wait for some event telling us that the click action was finished and we can check the telemetry results. But which one? - test that ACTIVE_SECONDS record is always present. - it's very suspicious that method checkResults is called before stopRecordingTelemetryLogs. We should check after the recording is finished.
Fixed the browser_telemetry_button_paintflashing.js test: - wait for the CLI command to be finished before doing anything else - using a commandOutputManager as used also in browser_telemetry_button_eyedropper.js - rewrite the delayedClicks function into a generator function - much better code without nested promises Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6ed69748707
Attachment #8621543 - Flags: review?(jwalker)
Attachment #8616613 - Attachment is obsolete: true
6 years ago
Attachment #8621543 - Flags: review?(jwalker) → review+
I found this issue in Nightly 41.0a1 (2015-05-29) (Build ID: 20150529030205) on Linux x64 This Bug is now Verified as fixed on Latest Firefox Beta 41.0b3 Build ID : 20150820142145 User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0
QA Whiteboard: [good first verify] → [good first verify][testday-20150821]
I have reproduced this bug in Nightly 41.0a1 (2015-05-29)(Build ID:20150529030205) with Comment 0's instruction on Windows 10 64bit. Bug is fixed now on latest Beta 41.0b3 (Build ID:20150820142145) Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0 [testday-20150821]
As it is verified on Linux and Windows (Comment 14 and Comment 15), Marking it as verified!
This is regression from bug 1128988 (between 2015-04-23 and 2015-04-24), and it wasn't completely fixed (see bug 1209149). Regression range: > https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b202671c9e2&tochange=22a157f7feb7
You need to log in before you can comment on or make changes to this bug.