Last Comment Bug 702342 - Filter Button should remain popped up when unchecking a suboption if any of the other suboptions remain checked
: Filter Button should remain popped up when unchecking a suboption if any of t...
Status: RESOLVED FIXED
[fixed-in-fx-team][qa+][qa!:10]
: verified-beta
Product: Firefox
Classification: Client Software
Component: Developer Tools: Console (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 11
Assigned To: Heather Arthur [:harth]
:
: Brian Grinstead [:bgrins]
Mentors:
Depends on:
Blocks: 703615
  Show dependency treegraph
 
Reported: 2011-11-14 10:37 PST by Heather Arthur [:harth]
Modified: 2012-01-05 13:39 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
verified


Attachments
Only uncheck filter button if all of it's suboptions get unchecked (1.18 KB, patch)
2011-11-14 10:37 PST, Heather Arthur [:harth]
mihai.sucan: feedback+
Details | Diff | Splinter Review
Only uncheck filter button if all of it's suboptions get unchecked, with tests fixed (5.29 KB, patch)
2011-11-22 14:07 PST, Heather Arthur [:harth]
no flags Details | Diff | Splinter Review
Only uncheck filter button if all of it's suboptions get unchecked, with tests fixed v2 (5.45 KB, patch)
2011-11-22 14:52 PST, Heather Arthur [:harth]
mihai.sucan: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Heather Arthur [:harth] 2011-11-14 10:37:14 PST
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.
Comment 1 Paul Rouget [:paul] 2011-11-18 10:05:14 PST
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] 2011-11-18 10:10:48 PST
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] 2011-11-18 10:11:55 PST
Also, thank you Heather for the patch and for the bug report!
Comment 4 Heather Arthur [:harth] 2011-11-18 11:21:45 PST
(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] 2011-11-22 14:07:44 PST
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.
Comment 6 Heather Arthur [:harth] 2011-11-22 14:52:40 PST
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.
Comment 7 Mihai Sucan [:msucan] 2011-11-23 10:48:10 PST
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] 2011-11-23 19:04:46 PST
http://hg.mozilla.org/integration/fx-team/rev/aa0b073a4ef6
Comment 9 Rob Campbell [:rc] (:robcee) 2011-11-24 08:25:38 PST
https://hg.mozilla.org/mozilla-central/rev/aa0b073a4ef6
Comment 10 Rob Campbell [:rc] (:robcee) 2011-11-24 08:33:59 PST
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 christian 2011-11-28 13:17:49 PST
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) 2011-11-29 06:11:14 PST
the flag and your words don't match up, Christian. Did you mean to approve this?
Comment 13 Rob Campbell [:rc] (:robcee) 2011-12-09 06:46:20 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/79a94c93dde8
Comment 14 Paul Silaghi, QA [:pauly] 2012-01-04 02:25:45 PST
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

Note You need to log in before you can comment on or make changes to this bug.