Closed Bug 1169667 Opened 10 years ago Closed 9 years ago

DevTools: when toggling the "Highlight painted area" tool, the icon color is wrong

Categories

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)

41 Branch
defect
Not set
normal

Tracking

(firefox38 unaffected, firefox39 unaffected, firefox40 affected, firefox41 verified)

VERIFIED FIXED
Firefox 41
Tracking Status
firefox38 --- unaffected
firefox39 --- unaffected
firefox40 --- affected
firefox41 --- verified

People

(Reporter: jsnajdr, Assigned: jsnajdr)

References

Details

(Keywords: regression, Whiteboard: [testday-20150821][regression from bug 1128988])

Attachments

(1 file, 1 obsolete file)

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
Status: UNCONFIRMED → NEW
Component: Untriaged → Developer Tools: Framework
Ever confirmed: true
OS: Unspecified → All
Hardware: Unspecified → All
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
Keywords: regression
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
Status: NEW → ASSIGNED
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)
Attachment #8616613 - Flags: review?(jwalker) → review+
Reminder to push to TRY when the tree re-opens.
Flags: needinfo?(pbrosset)
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.
Flags: needinfo?(pbrosset)
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
Flags: needinfo?(pbrosset)
Attachment #8621543 - Flags: review?(jwalker) → review+
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
QA Whiteboard: [good first verify]
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]
Whiteboard: [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!
Status: RESOLVED → 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
Blocks: 1128988
Whiteboard: [testday-20150821] → [testday-20150821][regression from bug 1128988]
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: