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

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Heather Arthur [:harth]]

Attachment: 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.

Comment 1

[Paul Rouget [:paul]]

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?

Comment 2

[Mihai Sucan [:msucan]]

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.

Comment 3

[Mihai Sucan [:msucan]]

Also, thank you Heather for the patch and for the bug report!

Comment 4

[Heather Arthur [:harth]]

(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.

Comment 5

[Heather Arthur [:harth]]

Attachment: 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.

Comment 6

[Heather Arthur [:harth]]

Attachment: 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.

Comment 7

[Mihai Sucan [:msucan]]

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!

Comment 8

[Heather Arthur [:harth]]

http://hg.mozilla.org/integration/fx-team/rev/aa0b073a4ef6

Comment 9

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/aa0b073a4ef6

Comment 10

[Rob Campbell [:rc] (:robcee)]

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.

Comment 11

[LegNeato]

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.

Comment 12

[Rob Campbell [:rc] (:robcee)]

the flag and your words don't match up, Christian. Did you mean to approve this?

Comment 13

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/79a94c93dde8

Comment 14

[Paul Silaghi, QA [:pauly]]

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