301562 - A > Z Sort Order is confusing, should be Ascending/Descending

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Meelad Zakaria]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050524 Fedora/1.0.4-4 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050524 Fedora/1.0.4-4 Firefox/1.0.4

in bookmark manager's view menu, there are too messages "A > Z sort order" and
"Z > A sort order". It's a mistype, cause "A>Z" looks like 'A' would come after
'Z', while it sorts bookmarks ascendingly and so it should be "A-Z sort order".
and the other one should be "Z-A sort order" as well.

Reproducible: Always

Steps to Reproduce:
1.Open firefox browser
2.Open bookmark manager
3.Go to view menu

Actual Results:  
wrong messages

Expected Results:  
right messages ;)

Comment 1

[Behnam Esfahbod [:zwnj]]

Or maybe "A -> Z" and "Z -> A"; I'm not sure...

Comment 2

[Nickolay_Ponomarev]

tweaking summary and confirming..

Comment 3

[u88484]

Attachment: patch

Patch changes 'A > Z sort order' to 'A -> Z sort order'

Comment 4

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

Comment on attachment 196419 [details] [diff] [review]
patch

These sort orders apply to all of the sort mechanisms, right? What does "A -> Z
Sort Order" mean when applied to "Sort by Date Added"?

If we're going to change this, let's change it to "Ascending Order" and
"Descending Order".

Comment 5

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

We might actually be able to steal the "Ascending" and "Descending" strings from
Thunderbird (View > Sort by menu) and try to avoid the l10n hit that way ...

Comment 6

[u88484]

Attachment: Patch 2.0

I don't follow what mike beltzner meant by steal the strings from TB. looks
like they name it the same way but then again i'm new to patching so I probably
just didn't know what exactly to look for with lxr.

Comment 7

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

Kurt, what I meant was there are already localized strings available for
"Ascending" and "Descending" in the Thunderbird product, so we might be able to
mitigate some of the l10n impact if we were to try to get this approved for 1.5.
It's a touch late for string changes, but that might actually make it happen.

One more nit (and sorry to do this again) that I forgot to tell you about. When
making changes to strings that affect the "meaning or format" the rule-of-thumb
is to change the keyname so that the localizers are alerted to the fact that
there was a string change. So, if you could change the keys to be:

<!ENTITY menuitem.view.ascendingOrder.label              "Ascending">
<!ENTITY menuitem.view.ascendingOrder.accesskey          "A">
<!ENTITY menuitem.view.descendingOrder.label             "Descending">
<!ENTITY menuitem.view.descendingOrder.accesskey         "D">

that'd be grand.

Comment 8

[u88484]

Attachment: Patch v3

Addresses Mike Beltzner's comments about using TB's localized strings.

Comment 9

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

Comment on attachment 197692 [details] [diff] [review]
Patch v3

Thanks Kurt. Any reason why you changed this in FFx's bookmark manager and SM's
history manager but not SM's bookmark manager?

Comment 10

[u88484]

Must have missed it, I will add a new patch for that later tonight or tomorrow.

Comment 11

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

Kurt: I don't think it's no-l10n impact, as they still *do* need to update the
strings with the new translation.

Pike: note, the new strings ("Ascending" and "Descending") exist in
Thunderbird's View menu, and so many localizers will already have translations
for them.

Comment 12

[u88484]

Attachment: follow up patch

Follow up patch to address comment# 9. How do I go about getting the firefox
portion landed on the trunk since denied for 1.8b5

Comment 13

[u88484]

Attachment: All-inclusive patch as requeste by gavin

Comment 14

[Asaf Romano (gone)]

Comment on attachment 198235 [details] [diff] [review]
All-inclusive patch as requeste by gavin

moving request to mconnor.

Comment 15

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

When I try to apply this patch to my tree I get an error saying that the patch
for sortFolder.dtd is malformed and ends unexpectedly in the middle of the line:

F:\dev\moz18>patch -p0 --dry-run < \dev\patches\301562-5.diff
patching file mozilla/browser/components/bookmarks/content/bookmarksManager.xul
patching file mozilla/browser/locales/en-US/chrome/browser/bookmarks/bookmarks.d
td
patching file mozilla/xpfe/components/history/resources/history.xul
patching file mozilla/xpfe/components/history/resources/locale/en-US/history.dtd

patching file mozilla/xpfe/components/bookmarks/resources/sortFolder.xul
patching file mozilla/xpfe/components/bookmarks/resources/locale/en-US/sortFolde
r.dtd
patch unexpectedly ends in middle of line
Hunk #1 FAILED at 8.
1 out of 1 hunk FAILED -- saving rejects to file mozilla/xpfe/components/bookmar
ks/resources/locale/en-US/sortFolder.dtd.rej

Comment 16

[u88484]

Donno why it fails...I only changed two lines and I didn't miss any < or "'s. I
started over again and patch came out exactly same as the patch you tried to
build with. 

Comment 17

[u88484]

Patch checked in by timeless:
mozilla/xpfe/components/bookmarks/resources/sortFolder.xul 1.5 	mozilla/xpfe/components/history/resources/localeen-US/history.dtd 1.17 
mozilla/xpfe/components/history/resources/history.xul 1.78

Comment 18

[Peter Weilbacher]

Seems like you forgot to checkin the change to
mozilla/xpfe/components/bookmarks/resources/locale/en-US/sortFolder.dtd
that was already part of the patch in attachment 198235 [details] [diff] [review] which now gave rise to bug 318866. Could you please fix that?

Comment 19

[Peter Weilbacher]

*** Bug 318866 has been marked as a duplicate of this bug. ***

Comment 20

[u88484]

CC timeless who checked this in

Comment 21

[u88484]

Sorry I meant I CCed timeless. Does another patch need submitted and reviewd? I'm not too sure what the process is what things like this happen.

Comment 22

[u88484]

Attachment: Patch to fix missed checkin

Guess I'll just attach this incase another patch is needed or if its quicker just to make a patch for what was forgotten to be checked in.

Comment 23

[u88484]

Attachment: Patch to fix missed checkin

Guess I'll just attach this incase another patch is needed or if its quicker just to make a patch for what was forgotten to be checked in.

Comment 24

[zug_treno]

*** Bug 319855 has been marked as a duplicate of this bug. ***

Comment 25

[Michael Graubart]

Re Comment #24: From the point of view of a user of SeaMonkey (and reporter of bugs) who is not familiar with the programming complexities, it seems that Bug 319855, though it may in some way be connected with Bug 301562, is not the same. It does not concern the sort-order, but the fact that sorting is actually impossible because instead of sorting, an error message appears. (Also, of course, Bug 319855 refers to SeaMonkey, not Firefox.)

Comment 26

[Asaf Romano (gone)]

Comment on attachment 204895 [details] [diff] [review]
Patch to fix missed checkin

Seamonkey patches on Seamonkey bugs please (plus, you need r/sr from neil or jag).

Comment 27

[Peter Weilbacher]

I agree that the XPFE portion of the patch doesn't belong here and that reviews were missing.

But now that the damage was done by checking that patch in only half-way it would still be nice to leave this bug open until the sortFolder.dtd part was checked in, too. Or the sortFolder.xul part backed out.

timeless or anybody else with a CVS account?

Comment 28

[u88484]

Comment on attachment 204895 [details] [diff] [review]
Patch to fix missed checkin

I've attached this patch in the original regression bug report, bug 318866

Comment 29

[:Gavin Sharp [email: [e-mail]]]

Attachment: Firefox branch patch

Comment 30

[O. Atsushi (Torisugari)]

It's not fixed in trunk Places...
http://lxr.mozilla.org/mozilla/source/browser/locales/en-US/chrome/browser/places/places.dtd#37

 33 <!ENTITY view.sortAscending.label
 34   "A > Z Sort Order">
 35 <!ENTITY view.sortAscending.accesskey
 36   "A">
 37 <!ENTITY view.sortDescending.label
 38   "Z > A Sort Order">
 39 <!ENTITY view.sortDescending.accesskey
 40   "Z">

-> reopen
or
-> file a new bug in "Places" category

...?

Comment 31

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/locales/en-US/chrome/browser/bookmarks/bookmarks.dtd 	1.9.4.1
mozilla/browser/components/bookmarks/content/bookmarksManager.xul 	1.29.4.3