424542 - Bookmarks manager needs a separate entry for Search:

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Ton [:Tonnes]]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.0.12) Gecko/20070508 Firefox/1.5.0.12
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT; nl; rv:1.9b5pre) Gecko/2008032106 Minefield/3.0b5pre

Until the upcoming release, the search.label entry in bookmarks.dtd was used for both the Search sidebar as well as the bookmarks organiser. This had not been a problem, as both terms (Search) were the same and basically meant ‘find’. In 3.0, places.dtd still uses the search.label, but its meaning has become different in the organizer window compared to the sidebar as the selectable places following the search string make it stand for places _where_ to search there, instead of only _what_ to search (i.o.w. find), like the latter is used in both the bookmarks and history sidebars.

In Dutch for instance, we changed the label (looking at its meaning in the organizer) from Zoeken to Doorzoeken (for German this would mean change Suchen to Durchsuchen). The logical result of this using a shared label is that Doorzoeken also appears in the sidebar, where we would still like to see Zoeken, as we still see in the history sidebar due to its own label (find.label in history.dtd).

It seems logical to me if the new entry would be the new one currently shown for the new meaning (Doorzoeken/Durchsuchen) in the organizer, keeping the old sidebar entry and its key as they were before (even though it would make sense to use a find.label/key for those). A new access key would not be necessary, as the current one is only being used by the sidebar.

If implementing this for 3.0 is a goal, copying search.label's content to the new entry for all locales may be a quick action; localizers then can see if they want to change the content of one of them or not. Using history.dtd's find.label for the bookmarks sidebar may even be a quicker option without having the need to bother localizers, though a little risky and not a very nice one of course.

Impact:
May look minor, but compared to older versions a visible regression if search.label has already been changed for locales. Basically needed anyway if the selection buttons remain in, or it may look unfinished.


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Ton [:Tonnes]]

Attachment: Screenshot of current situation

As requested by Pike, to make things clearer (especially for Mac users).

Comment 2

[Axel Hecht [:Pike]]

Mike, this seems to be a problem with "searching in ..." vs "searching for ...", which, for once in our life, don't affect Polish, but German and Dutch.

There is bug 422977 about removing the places query builder, setting a dependency on that.

Comment 3

[Mike Connor [:mconnor]]

Not an issue once we remove the query builder, will see what we end up with for v.next.

Comment 4

[Ton [:Tonnes]]

 (In reply to comment #3)
> Not an issue once we remove the query builder, will see what we end up with for
> v.next.

Although the query builder has gone, the Search button in the library hasn’t..

Comment 5

[Ton [:Tonnes]]

Unfortunately not much has happened so far, so I was wondering if it’s too late to get this into 3.5? It can’t be that hard to either create a second search*.label entry or always use history’s find.label in the sidebar, can it?

Comment 6

[Shawn Wilsher :sdwilsh]

Yes, it's too late for 3.5 for any new strings.

Comment 7

[Mike Connor [:mconnor]]

Let's put this on the blocking list for v.next, since this is a UX issue that should be easy to solve and improves quality in some key localizations.

Comment 8

[Curtis Bartley [:cbartley]]

Taking this bug.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

> Although the query builder has gone, the Search button in the library hasn’t..

Yes it has, it's now a search field without an external button. Does that not make this bug INVALID?

Any new string work would need to be completed by September 14th if it's to make Firefox 3.6, by the way.

Comment 10

[Marco Bonardo [:mak]]

(In reply to comment #9)
> > Although the query builder has gone, the Search button in the library hasn’t..
> 
> Yes it has, it's now a search field without an external button. Does that not
> make this bug INVALID?

when you search we still show the scope bar, so i suppose the bug applies to the Search: label in the scope bar. At least looking at screenshot in comment 1

Comment 11

[Ton [:Tonnes]]

(In reply to comment #10)
> (In reply to comment #9)
> > > Although the query builder has gone, the Search button in the library hasn’t..
> > 
> > Yes it has, it's now a search field without an external button. Does that not
> > make this bug INVALID?
> 
> when you search we still show the scope bar, so i suppose the bug applies to
> the Search: label in the scope bar. At least looking at screenshot in comment 1

Correct, this is about the Search label that currently shows up after entering a search phrase. So definetely not invalid IMO.

Comment 12

[Marco Bonardo [:mak]]

Curtis, this should land today for the string freeze, or won't block anymore.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Missed string freeze, not a blocker anymore.

Comment 14

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv

Comment 15

[Dietrich Ayala (:dietrich)]

This is not a blocker for 1.9.3. Would take a fix before whenever string freeze is.

Comment 16

[Ton [:Tonnes]]

As this bug will affect Fx4 as well and is likely to get of the radar (again), would this be a good time to impement this?

Comment 17

[Axel Hecht [:Pike]]

We're really close to string freeze once more, and I guess it won't block fx4.

If we'd get a patch real quick, we might be able to slip it in, though.

Comment 18

[Marco Bonardo [:mak]]

omg, we still have locale/browser/history/history.dtd that is completely nonsense (sounds like inheritance from FX2). Half of those entries are unused, and the remaining ones should be merged to places.dtd.

Pike, would you accept such a change or should I limit to add a search.in.label and file a follow-up to kill history/history.dtd post FX4?

Comment 19

[Marco Bonardo [:mak]]

Attachment: full patch v1.0

this would be the full patch, otherwise I'd just introduce search.in.label.
I'm still rebuilding to test the patch, but it should be fine.

Comment 20

[Axel Hecht [:Pike]]

Comment on attachment 485703 [details] [diff] [review]
full patch v1.0

Kinda sad either way, now is better than a day before the next string freeze ;-)

Comment 21

[Marco Bonardo [:mak]]

Comment on attachment 485703 [details] [diff] [review]
full patch v1.0

Axel, once reviewed, do I need approval for b7 or is b8 fine?

Comment 22

[Dietrich Ayala (:dietrich)]

Comment on attachment 485703 [details] [diff] [review]
full patch v1.0

r+ assuming confirmation that these removed strings aren't used anymore.

Comment 23

[Marco Bonardo [:mak]]

(In reply to comment #22)
> r+ assuming confirmation that these removed strings aren't used anymore.

I did a search for each one in mxr, and manually tested history sidebar, without noticing any glitch or missing string.

Comment 24

[Axel Hecht [:Pike]]

We can't land this on just b8 unless we built b7. So either both, or wait 'til b7 is out the door.

Comment 25

[Marco Bonardo [:mak]]

Comment on attachment 485703 [details] [diff] [review]
full patch v1.0

ok ,then asking review to land on central and b7, or otherwise, if we feel better waiting, only for central once b7 is out (not sure why we can't land on central while b7 is not out, is this related to issues with l10n dashboards distinguishing branches?).
Please specify what you prefer me doing.

Comment 26

[Mike Beltzner [:beltzner, not reading bugmail]]

a=beltzner for mozilla-central default and GECKO20b7pre_20101006_RELBRANCH

Comment 27

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/3d56a8182a7a

Looks like we don't have anymore to land to b7 branch from what I was told. Let me know if i'm wrong.

Comment 28

[Mike Beltzner [:beltzner, not reading bugmail]]

> Looks like we don't have anymore to land to b7 branch from what I was told. Let
> me know if i'm wrong.

You're not wrong. I'll post to dev-planning about this soon.

Comment 29

[Marek Stępień [:marcoos, inactive]]

(In reply to comment #2)
> Mike, this seems to be a problem with "searching in ..." vs "searching for
> ...", which, for once in our life, don't affect Polish, but German and Dutch. 

FWIW, it affects Polish, too. "Search in $foo" would be "Przeszukaj $foo" or "Szukaj w $foo" and "Search for $foo" would be "Szukaj $foo" / "Wyszukaj $foo" / "Znajdź $foo".

The form we used until now ("Przeszukaj:") ended with a colon which somehow made it kind of (but not really) fit both situations.

Now it's much better. Thank you, Marco, for fixing this. :)

Comment 30

[Marco Bonardo [:mak]]

you're welcome, keep up the good work with localization :)

Comment 31

[Ton [:Tonnes]]

Thanks all. :)