Filter Button should remain popped up when unchecking a suboption if any of the other suboptions remain checked

RESOLVED FIXED in Firefox 10

Status

()

Firefox
Developer Tools: Console
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: harth, Assigned: harth)

Tracking

({verified-beta})

unspecified
Firefox 11
verified-beta
Points:
---

Firefox Tracking Flags

(firefox10- verified)

Details

(Whiteboard: [fixed-in-fx-team][qa+][qa!:10])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 574339 [details] [diff] [review]
Only uncheck filter button if all of it's suboptions get unchecked

The behavior of the filter buttons when you uncheck a "Warning" or "Error" selection is confusing, the STR:

1. Open up http://harthur.github.com/test-pages/filters.html
2. Open the Web Console
3. Reload the page so you see the errors and log messages
4. Make sure the "JS" filter button is "On" (all options checked, button depressed)
5. Click the drop-down of the "JS" button
6. Select the "Warnings" option to filter out JS warnings

Actual:
the "document.all" warning is no longer shown, and the "JS" button looks "off" (popped-up)

Expected:
the "document.all" warning is no longer shown, but the "JS" button still looks "on" (depressed).


I'm often turning off just warnings in the web console and the filter UI has confused the living daylights out of me, and I think this is why. When I'm seeing "JS stuff" in the web console, I want the button UI to reflect this.

There's bug 626018 about making a tri-state UI for the filter buttons (all on, some on, none on), but in the meantime I think we could do this. Here's a patch if you want to play around, let me know what you think of this idea.
(Assignee)

Updated

6 years ago
Attachment #574339 - Attachment is patch: true

Comment 1

6 years ago
Comment on attachment 574339 [details] [diff] [review]
Only uncheck filter button if all of it's suboptions get unchecked

Looks good. Did you try to run the tests?
Attachment #574339 - Flags: review?(mihai.sucan)
Comment on attachment 574339 [details] [diff] [review]
Only uncheck filter button if all of it's suboptions get unchecked

Review of attachment 574339 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks good, indeed. We should probably have a test that makes sure the UI works as expected.
Attachment #574339 - Flags: review?(mihai.sucan) → feedback+
Also, thank you Heather for the patch and for the bug report!

Updated

6 years ago
Blocks: 703615
(Assignee)

Comment 4

6 years ago
(In reply to Paul Rouget [:paul] from comment #1)
> Comment on attachment 574339 [details] [diff] [review] [diff] [details] [review]
> Only uncheck filter button if all of it's suboptions get unchecked
> 
> Looks good. Did you try to run the tests?

Uh, yeah, good point (: the tests totally fail. Let me upload a new patch and ask for review.

Updated

6 years ago
Component: Developer Tools → Developer Tools: Console
QA Contact: developer.tools → developer.tools.console
(Assignee)

Comment 5

6 years ago
Created attachment 576255 [details] [diff] [review]
Only uncheck filter button if all of it's suboptions get unchecked, with tests fixed

Patch as above, but with the fixed up filter button tests passing.
Assignee: nobody → fayearthur
Attachment #574339 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #576255 - Flags: review?(mihai.sucan)
(Assignee)

Comment 6

6 years ago
Created attachment 576312 [details] [diff] [review]
Only uncheck filter button if all of it's suboptions get unchecked, with tests fixed v2

Actually, going to add in a test for the case where you uncheck all the suboptions (the main button becomes unchecked in this case), so we know if we change this behavior.
Attachment #576255 - Attachment is obsolete: true
Attachment #576255 - Flags: review?(mihai.sucan)
Attachment #576312 - Flags: review?(mihai.sucan)
Comment on attachment 576312 [details] [diff] [review]
Only uncheck filter button if all of it's suboptions get unchecked, with tests fixed v2

Review of attachment 576312 [details] [diff] [review]:
-----------------------------------------------------------------

Great! Thank you Heather!
Attachment #576312 - Flags: review?(mihai.sucan) → review+
(Assignee)

Comment 8

6 years ago
http://hg.mozilla.org/integration/fx-team/rev/aa0b073a4ef6
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/aa0b073a4ef6
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
tracking-firefox10: --- → ?
Comment on attachment 576312 [details] [diff] [review]
Only uncheck filter button if all of it's suboptions get unchecked, with tests fixed v2

low risk, nice fix for a mildly-annoying visual bug.
Attachment #576312 - Flags: approval-mozilla-aurora?

Comment 11

6 years ago
Comment on attachment 576312 [details] [diff] [review]
Only uncheck filter button if all of it's suboptions get unchecked, with tests fixed v2

[triage comment]
This generally wouldn't meet the bar for aurora, but we'll take it as it isn't late in the cycle and regressions will generally be easy to spot.
Attachment #576312 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
the flag and your words don't match up, Christian. Did you mean to approve this?
Attachment #576312 - Flags: approval-mozilla-aurora- → approval-mozilla-aurora?

Updated

6 years ago
Attachment #576312 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/79a94c93dde8
status-firefox10: --- → fixed
Whiteboard: [fixed-in-fx-team] → [fixed-in-fx-team][qa+]
This is verified fixed on Firefox 10 Beta2:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0) Gecko/20100101 Firefox/10.0
status-firefox10: fixed → verified
Keywords: verified-beta
Whiteboard: [fixed-in-fx-team][qa+] → [fixed-in-fx-team][qa+][qa!:10]

Updated

6 years ago
tracking-firefox10: ? → -
You need to log in before you can comment on or make changes to this bug.