Last Comment Bug 715892 - In the Style Editor, remove the stylesheet filter
: In the Style Editor, remove the stylesheet filter
Status: VERIFIED FIXED
[qa!]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Style Editor (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Firefox 12
Assigned To: Cedric Vivier [:cedricv]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-06 07:16 PST by Paul Rouget [:paul]
Modified: 2012-03-23 06:06 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified
verified


Attachments
patch v1 (13.48 KB, patch)
2012-01-23 08:35 PST, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
patch v1.1 (14.62 KB, patch)
2012-01-23 08:41 PST, Paul Rouget [:paul]
dcamp: review+
cedricv: feedback+
Details | Diff | Splinter Review
screencast (see comment 11) (1007.99 KB, video/webm)
2012-01-24 09:52 PST, Paul Rouget [:paul]
no flags Details
patch v2 - rebased against tip and micro fixes (17.22 KB, patch)
2012-01-25 01:40 PST, Cedric Vivier [:cedricv]
no flags Details | Diff | Splinter Review
patch v2 - remove more unused entities from dtd (17.44 KB, patch)
2012-01-25 01:45 PST, Cedric Vivier [:cedricv]
no flags Details | Diff | Splinter Review
patch v3 - resizer bug fixed (!?) (17.39 KB, patch)
2012-01-25 02:08 PST, Cedric Vivier [:cedricv]
no flags Details | Diff | Splinter Review
patch v3.1 - better id naming (17.39 KB, patch)
2012-01-25 02:12 PST, Cedric Vivier [:cedricv]
paul: review+
Details | Diff | Splinter Review
patch v3.2 - id name change + move persist attribute as well (17.42 KB, patch)
2012-01-25 04:02 PST, Cedric Vivier [:cedricv]
paul: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Paul Rouget [:paul] 2012-01-06 07:16:16 PST
This is not really useful, and can be a bit confusing.
Comment 1 Cedric Vivier [:cedricv] 2012-01-19 05:55:02 PST
As discussed, we'll change the behavior to a more useful "Find in all style sheets" (bug 719409)
Comment 2 Paul Rouget [:paul] 2012-01-23 08:00:28 PST
Bug 719409 won't be fixed for Firefox 11. To avoid confusion, I think we should remove this feature in Firefox 11.
Comment 3 Paul Rouget [:paul] 2012-01-23 08:35:22 PST
Created attachment 590724 [details] [diff] [review]
patch v1
Comment 4 Paul Rouget [:paul] 2012-01-23 08:39:28 PST
The logic behind this deletion is:
- the filter-by-name feature is not useful
- we want to replace the behavior of the search field from filter-by-name to filter-by-content
- this won't happen for Firefox 11 or Firefox 12
- we don't want people to get used to a feature that will be removed and replaced by something that "look" the same but won't do the same thing
Comment 5 Paul Rouget [:paul] 2012-01-23 08:41:45 PST
Created attachment 590726 [details] [diff] [review]
patch v1.1
Comment 6 Paul Rouget [:paul] 2012-01-23 09:13:36 PST
Comment on attachment 590726 [details] [diff] [review]
patch v1.1

Asking dcamp to review to make sure we can land that asap.
But we still need a f+ from Cedric.
Comment 7 Cedric Vivier [:cedricv] 2012-01-23 13:48:02 PST
Comment on attachment 590726 [details] [diff] [review]
patch v1.1

f- because I actually want to say "f+ but...

...ideally, we could still keep the "search-on-type" feature even so we remove it the visible searchbox for Fx11 :)
Comment 8 Paul Rouget [:paul] 2012-01-23 14:52:10 PST
I don't think we want to keep dead code. You can re-introduce this code if needed.
Comment 9 Cedric Vivier [:cedricv] 2012-01-23 17:12:42 PST
(In reply to Paul Rouget [:paul] from comment #8)
> I don't think we want to keep dead code. You can re-introduce this code if
> needed.

I meant I think we should keep the logic AND the "search-on-type" UI (you can try it, eg., when the Style Editor is in narrow in vertical side bar)
Comment 10 Cedric Vivier [:cedricv] 2012-01-24 02:06:17 PST
Comment on attachment 590726 [details] [diff] [review]
patch v1.1

So as discussed on IRC, we'll rather improve the search-on-type filter and land it together with the new global search box later.
Comment 11 Paul Rouget [:paul] 2012-01-24 09:50:08 PST
So I run into an issue. With this patch, we can actually resize the right panel in a way that the buttons get hidden.

If, in the toolbar, I add a <xul:textbox flex="1"> (after the button, or between the buttons), the toolbar can't get over-resized.

I really don't understand what is going on. I need some help here.
Comment 12 Paul Rouget [:paul] 2012-01-24 09:52:17 PST
Created attachment 591144 [details]
screencast (see comment 11)
Comment 13 Cedric Vivier [:cedricv] 2012-01-25 01:40:50 PST
Created attachment 591391 [details] [diff] [review]
patch v2 - rebased against tip and micro fixes

Paul, I had a look at it... tried few things but no idea what is going on with that resizer :/
Anyways I rebased the patch to apply against tip.
Comment 14 Cedric Vivier [:cedricv] 2012-01-25 01:45:09 PST
Created attachment 591392 [details] [diff] [review]
patch v2 - remove more unused entities from dtd
Comment 15 Cedric Vivier [:cedricv] 2012-01-25 02:08:40 PST
Created attachment 591394 [details] [diff] [review]
patch v3 - resizer bug fixed (!?)

Seems to work now. Yay!
Comment 16 Cedric Vivier [:cedricv] 2012-01-25 02:12:25 PST
Created attachment 591395 [details] [diff] [review]
patch v3.1 - better id naming
Comment 17 Paul Rouget [:paul] 2012-01-25 03:47:10 PST
Comment on attachment 591395 [details] [diff] [review]
patch v3.1 - better id naming

Thank you Cedric. Can I just ask you to find a better "id" for splitview-nav-container? It's a little confusing to get the same className and Id.
Comment 18 Cedric Vivier [:cedricv] 2012-01-25 04:02:43 PST
Created attachment 591407 [details] [diff] [review]
patch v3.2 - id name change + move persist attribute as well
Comment 20 Paul Rouget [:paul] 2012-01-25 05:36:42 PST
I will a? this patch once in central.
Comment 21 Tim Taubert [:ttaubert] 2012-01-25 09:06:37 PST
https://hg.mozilla.org/mozilla-central/rev/ffaed1950e3b
Comment 22 Rob Campbell [:rc] (:robcee) 2012-01-25 09:53:52 PST
Comment on attachment 591407 [details] [diff] [review]
patch v3.2 - id name change + move persist attribute as well

[Approval Request Comment]
Regression caused by (bug #): Not a regression. New feature.
User impact if declined: Confusing Feature will Confuse Users, some window repositioning behavior may not work as expected.
Testing completed (on m-c, etc.): on m-c, local testing.
Risk to taking this patch (and alternatives if risky): negligible. Patch removes more code than adds. Clean!
Comment 23 Alex Keybl [:akeybl] 2012-01-25 18:29:56 PST
Comment on attachment 591407 [details] [diff] [review]
patch v3.2 - id name change + move persist attribute as well

[Triage Comment]
It's difficult to argue with the removal of functionality in a new feature for the sake of minimizing user confusion. Approved for Aurora.
Comment 24 Rob Campbell [:rc] (:robcee) 2012-01-26 12:47:09 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/a8d8390ebb52
Comment 25 Simona B [:simonab] 2012-02-07 05:29:34 PST
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0

Verified using Firefox 11 beta 1 on Windows 7, Ubuntu 11.10 and Mac OS X 10.6 that the stylesheet filter has been removed from the Style Editor.
Comment 26 Simona B [:simonab] 2012-03-23 06:06:07 PDT
Verified as fixed on Firefox 12 beta 2 - the stylesheet filter has been removed from the Style Editor.

Verified on Windows 7, Ubuntu 11.10 and Mac OS X 10.6.
Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0

Setting resolution to VERIFIED FIXED.

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