Closed Bug 1272903 Opened 3 years ago Closed 3 years ago

Wrong/missing comment for localization note notADeceptiveSite, notAnAttack

Categories

(SeaMonkey :: General, defect, trivial)

SeaMonkey 2.45 Branch
defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.46

People

(Reporter: nONoNonO, Assigned: nONoNonO)

References

Details

Attachments

(1 file, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #1272901 +++

In bug 1245992 web forgery was renamed to deceptive sites, but in the localization comment for browser it still reads: LOCALIZATION NOTE (notAForgery, notAnAttack).
https://hg.mozilla.org/mozilla-central/rev/4e157cde861c#l6.7

For suite in bug 1250600 the comment is changed, but the next two lines explaining the note are removed, making the comment bogus.
https://hg.mozilla.org/comm-central/rev/1759686baf84#l8.15
Attached patch bug1272903.patch (obsolete) — Splinter Review
Attachment #8752515 - Flags: review?(acelists)
Assignee: acelists → o.e.ekker
Status: NEW → ASSIGNED
Attachment #8752515 - Flags: review?(acelists) → review?(philip.chee)
Comment on attachment 8752515 [details] [diff] [review]
bug1272903.patch

I think that was me. Removed the keys there but forgot to adjust the comment. This should be completely removed from notification.properties.

In safeBrowsing.dtd it should be:

>> <!-- LOCALIZATION NOTE (reportDeceptiveSite, notADeceptiveSite)
>>     The two button strings will never be shown at the same time,
>>     so it's okay for them to have the same access key. -->
Attachment #8752515 - Flags: feedback-
This change is in browser.properties . See bug 1272901 and bug 1245992.
See Bug 1250600. The keys in notification.properties are and were different. The localization note there didn't even make sense before the patch. In safebrowsing.dtd they are the same.
Well, the keys can be the same in other locales, so if the buttons arrn't shown at the same time the comment is probably needed in both places.
We discussed this on IRC today and think the comment makes sense if it means localizers can make the access keys the same if they want to. Also Firefox still has the comment in browser.properties near stings that look the same as in Seamonkey's notification.properties. So is it NOT true that those access keys can be the same for Seamonkey only? Are these strings notification.properties used differently than in browser.properties?
Yupp you are right. If you look at it this way in other languages it can/might be the same key. But could you still add the comment in safebbrowsing.dtd to make it more complete. There it's more likely that always the same key is used.
Attached file patch v2 (obsolete) —
OK, thanks.
Attachment #8752515 - Attachment is obsolete: true
Attachment #8752515 - Flags: review?(philip.chee)
Attachment #8752524 - Flags: review?(philip.chee)
Comment on attachment 8752524 [details]
patch v2

I think it's time for FRG to start doing reviews ;)
Attachment #8752524 - Flags: review?(philip.chee) → review?(frgrahl)
Comment on attachment 8752524 [details]
patch v2

Looks like ratty think I can't screw a 5 line review lines up. Little does he know :)

Back to business:

r+ from me if you fix one small NIT. Please end the first comment with a period like the second.
Attachment #8752524 - Flags: review?(frgrahl) → review+
Hmz... my nit would be that the second comment should be on one line, like the other comments in that file :--)
Attached patch patch v3Splinter Review
OK:)
Attachment #8752524 - Attachment is obsolete: true
Attachment #8753016 - Flags: review+
Keywords: checkin-needed
>> Hmz... my nit would be that the second comment should be on one line, like the other comments in that file :--)

Well the point was important or Ratty and Ian would have yelled (rightly) at me :) 

Before we start a formatting debate over a comment I overstep my authority and git it an a=me too so that we can close this bug soon.
I have incorporated both nits as they seemed useful :)

https://hg.mozilla.org/comm-central/rev/ad4547cf143742a3266eeeb8f2549d51e2cb4107
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.46
You need to log in before you can comment on or make changes to this bug.