133119 - Language in Image Acceptance Policy Pref Page Should Be Consistent

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect, P4)

Description

[Felix Miata]

2002032316 OS/2

Let's use either load or accept, not both. Ask me should be either "before
loading" or "before accepting", according to whether load or accept is the
appropriate word to use, and not "downloading".

Comment 1

[timeless]

.

Comment 2

[Henrik Gemal]

currently we have:
"do not load any images"
"accept images that comes..."
"accept all images"

so yes, lets be consistent. either use "load" or use "accept"

Comment 3

[Mitchell Stoltz (not reading bugmail)]

Mass reassigning of Image manager bugs to [e-mail], and futuring.
Most of these bugs are enhancement requests or are otherwise low priority at
this time.

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - change pref dialog text and help page

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Reassigning to Prefs for review.

Comment 6

[Serge Gautherie (:sgautherie)]

(Filter "spam" on 'prefs-nobody-20080612'.)

Comment 7

[Philip Chee]

Comment on attachment 126639 [details] [diff] [review]
Patch - change pref dialog text and help page

triage: Probably WONTFIX but if not definitely needs to be un-bitrotted.

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 126639 [details] [diff] [review]
Patch - change pref dialog text and help page

Trying to achieve consistency is good, so let's do this. The dialog behind the Manage Image Permissions uses "load", too, and I think it makes sense (if we accept images we also display them, right?).

"Accept images that come from the originating server only" sounds strange to me for another reason: it lays too much stress on "only" for my taste, as if only images that could not be found elsewhere would be allowed. Maybe a native speaker can comment on whether it's just me.

I'd suggest:
* Do not load any images
* Only load images from the originating server
* Load all images

The accesskeys probably need to be changed according to the final wording.

Since this is a UI change, consulting Neil is mandatory. But I guess that can wait until someone actually uploads a new patch.

Help looks differently today as well (it should describe all the options 1:1) so it needs to be changed accordingly (the part in the old patch is out-of-date).

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

Oh, I forgot: FF only has "Load images automatically" under Options/Content.

Comment 10

[Ian Neal]

Comment on attachment 126639 [details] [diff] [review]
Patch - change pref dialog text and help page

I'm happy with Jens suggestion, though do we need to actually say "images" in any of the options as the starting statement is:
"Specify how SeaMonkey handles images."
As already mentioned patch has bitrotted.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #10)
> do we need to actually say "images" in
> any of the options as the starting statement is:
> "Specify how SeaMonkey handles images."

Hmm, my first thought was "why not, shorter is better, let's change it". Then I thought that this might cause a11y issues. I'm still not 100% sure it won't but I found that the other setting on the Images pref pane, "Animated images should loop", is already in short form ("As many times as the image specifies", "Once", "Never"). The difference here is that the introduction matches the options.

How about:

Load images from
( ) No server (block all images)
( ) The originating server only
(*) Any server

Comment 12

[Edmund Wong (:ewong)]

(In reply to comment #11)
> (In reply to comment #10)
> > do we need to actually say "images" in
> > any of the options as the starting statement is:
> > "Specify how SeaMonkey handles images."
> 
> Hmm, my first thought was "why not, shorter is better, let's change it". Then I
> thought that this might cause a11y issues. I'm still not 100% sure it won't but
> I found that the other setting on the Images pref pane, "Animated images should
> loop", is already in short form ("As many times as the image specifies",
> "Once", "Never"). The difference here is that the introduction matches the
> options.
> 
> How about:
> 
> Load images from
> ( ) No server (block all images)
> ( ) The originating server only
> (*) Any server

I think I like your original suggestion.  As in:

* Do not load any images
* Only load images from the originating server
* Load all images

Comment 13

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #12)
> I think I like your original suggestion.  As in:

Feel free to take, but if you do, make sure to request b3 blocking (l10n impact) and file a Help bug in the end. Thanks!

Comment 14

[Edmund Wong (:ewong)]

Attachment: Made the Image acceptance policy pref page more language consistent.

Comment 15

[Philip Chee]

First you need to get a r+ then you ask for blocking?

Comment 16

[Ian Neal]

Comment on attachment 519124 [details] [diff] [review]
Made the Image acceptance policy pref page more language consistent.

>+<!ENTITY accAllImagesRadio.label        "Load all images">
>+<!ENTITY accAllImagesRadio.accesskey    "d">
I would use "L" here
>+<!ENTITY accOrgImagesRadio.label        "Only load images that come from the originating server">
>+<!ENTITY accOrgImagesRadio.accesskey    "L">
I would use "n" here
> <!ENTITY disableImages.label            "Do not load any images">
> <!ENTITY disableImages.accesskey        "n">
Change this to "D"
> <!ENTITY viewPermissions.label          "Manage Permissions">
> <!ENTITY viewPermissions.accesskey      "P">

I still think we are needlessly repeating "load images" unfortunately I can not see a way of expressing it in a natural sounding manner. Perhaps Neil might if you request feedback/ui-review on the new patch?

Don't forget help needs updating as the original patch did that.

r- for the moment as new patch (with help changes) will need a review.

Comment 17

[Edmund Wong (:ewong)]

Attachment: Made the Image acceptance policy pref page more language consistent. (v2) [Checkin: comment 18]

Comment 18

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 519637 [details] [diff] [review]
Made the Image acceptance policy pref page more language consistent. (v2) [Checkin: comment 18]

http://hg.mozilla.org/comm-central/rev/a0aca3c04ef3

Comment 19

[Jens Hatlak (:InvisibleSmiley)]

Leaving open for now since an answer for how to proceed with Help changes is needed (in this bug or a follow-up?).

Comment 20

[Giacomo Magnini]

The rule under FF is that if you change the string, you must also change the entity, to help localizers in their task: can we please stick to it also for SM?
I was lucky to read the checkin, so I spotted it easily, but I guess not many other localizers have seen that.

Comment 21

[Edmund Wong (:ewong)]

While I'm changing the entities, I'll be doing the help on a separate patch
that I will attach to this bug.

Comment 22

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #20)
> The rule under FF is that if you change the string, you must also change the
> entity, to help localizers in their task

That rule also applies to SM in general. Even though I was not the reviewer here I thought that it didn't really apply here since it's just a clarification of expressions in English rather than a fundamental change of meaning. On second thought, maybe the change from Accept to Load justifies a change of entity names, though.

Comment 23

[Justin Wood (:Callek)]

(In reply to comment #20)
> The rule under FF is that if you change the string, you must also change the
> entity, to help localizers in their task: can we please stick to it also for
> SM?
> I was lucky to read the checkin, so I spotted it easily, but I guess not many
> other localizers have seen that.

I wouldn't have blocked for this bug; but since the l10n changes landed without entity changes I will.

I would ask the help changes go to a followup for easier tracking, since the first part of this bug already landed. And tracking one bug for disjoint releases is a pain.

Comment 24

[Edmund Wong (:ewong)]

Attachment: Language in Image Acceptance Policy pref page should be consistent. (v3)

This patch includes entity changes so it requires a re-review.

Comment 25

[Edmund Wong (:ewong)]

I have quite confused myself.  I submitted a new patch but realized that 
the old patch (v2) was already committed.  So. I will spin off two new
bugs that will.  1) Cover the entity changes I had planned. (bug #643676)
                 2) Fix the help. (bug #643677)

Comment 26

[Justin Wood (:Callek)]

(In reply to comment #23)
> (In reply to comment #20)
> > The rule under FF is that if you change the string, you must also change the
> > entity, to help localizers in their task: can we please stick to it also for
> > SM?
> > I was lucky to read the checkin, so I spotted it easily, but I guess not many
> > other localizers have seen that.
> 
> I wouldn't have blocked for this bug; but since the l10n changes landed without
> entity changes I will.
> 

Moved to Bug 643676