715892 - In the Style Editor, remove the stylesheet filter

Status: VERIFIED FIXED

(DevTools :: Style Editor, defect)

Description

[Paul Rouget [:paul]]

This is not really useful, and can be a bit confusing.

Comment 1

[Cedric Vivier [:cedricv]]

As discussed, we'll change the behavior to a more useful "Find in all style sheets" (bug 719409)

Comment 2

[Paul Rouget [:paul]]

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]]

Attachment: patch v1

Comment 4

[Paul Rouget [:paul]]

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]]

Attachment: patch v1.1

Comment 6

[Paul Rouget [:paul]]

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]]

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]]

I don't think we want to keep dead code. You can re-introduce this code if needed.

Comment 9

[Cedric Vivier [:cedricv]]

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

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]]

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]]

Attachment: screencast (see comment 11)

Comment 13

[Cedric Vivier [:cedricv]]

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

Attachment: patch v2 - remove more unused entities from dtd

Comment 15

[Cedric Vivier [:cedricv]]

Attachment: patch v3 - resizer bug fixed (!?)

Seems to work now. Yay!

Comment 16

[Cedric Vivier [:cedricv]]

Attachment: patch v3.1 - better id naming

Comment 17

[Paul Rouget [:paul]]

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]]

Attachment: patch v3.2 - id name change + move persist attribute as well

Comment 19

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/ffaed1950e3b

Comment 20

[Paul Rouget [:paul]]

I will a? this patch once in central.

Comment 21

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 22

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

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]]

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)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a8d8390ebb52

Comment 25

[Simona Badau, Desktop QA]

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 Badau, Desktop QA]

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.