Show warning text in the user settings page for non-owner users.

Show warning text in the user settings page for non-owner users.

The user settings may only be modified by the owner. We should
show a notice about it. This is based on kenmoore's mock.

The image was converted from phishing_icon.png:

% convert -geometry 17x17 ../shared/images/phishing_icon.png  warning.png

BUG=chromium-os:9254
TEST=confirmed that the warning text was shown for a non-owner and the guest.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=69812

[kochi, 2010-12-20 08:00:18]

http://codereview.chromium.org/5965003/diff/1/chrome/app/generated_resources.grd
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/5965003/diff/1/chrome/app/generated_resources....
chrome/app/generated_resources.grd:9494: <message
name="IDS_OPTIONS_ACCOUNTS_OWNER_ONLY" desc="In the Accounts settings tab, the
warning text noticing that the only owner can modify these settings.">
s/the only/only the/ sounds more natural to me (but may be wrong, I'm not a
native speaker.)

http://codereview.chromium.org/5965003/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/about_page.js (left):

http://codereview.chromium.org/5965003/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/about_page.js:157: $('updateIcon').className=
'update-icon ' + state;
What is this change?

http://codereview.chromium.org/5965003/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/chromeos_accounts_options.js (right):

http://codereview.chromium.org/5965003/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/chromeos_accounts_options.js:46:
$('ownerOnlyWarning').style.display = 'block';
Could you use CSS class "hidden" for this instead of
directly manipulating the style?

Setting the class="hidden" on the html element by default
and

$('ownerOnlyWarning').classList.remove('hidden');

here would work, I think.

(The .hidden class is defined in options_page.css)

[satorux1, 2010-12-20 08:16:14]

http://codereview.chromium.org/5965003/diff/1/chrome/app/generated_resources.grd
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/5965003/diff/1/chrome/app/generated_resources....
chrome/app/generated_resources.grd:9494: <message
name="IDS_OPTIONS_ACCOUNTS_OWNER_ONLY" desc="In the Accounts settings tab, the
warning text noticing that the only owner can modify these settings.">
On 2010/12/20 08:00:18, Takayoshi Kochi wrote:
> s/the only/only the/ sounds more natural to me (but may be wrong, I'm not a
> native speaker.)

Done.

http://codereview.chromium.org/5965003/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/about_page.js (left):

http://codereview.chromium.org/5965003/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/about_page.js:157: $('updateIcon').className=
'update-icon ' + state;
On 2010/12/20 08:00:18, Takayoshi Kochi wrote:
> What is this change?

Good catch. This was an editing error. Fixed.

http://codereview.chromium.org/5965003/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/chromeos_accounts_options.js (right):

http://codereview.chromium.org/5965003/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/chromeos_accounts_options.js:46:
$('ownerOnlyWarning').style.display = 'block';
On 2010/12/20 08:00:18, Takayoshi Kochi wrote:
> Could you use CSS class "hidden" for this instead of
> directly manipulating the style?
> 
> Setting the class="hidden" on the html element by default
> and
> 
> $('ownerOnlyWarning').classList.remove('hidden');
> 
> here would work, I think.
> 
> (The .hidden class is defined in options_page.css)

Good idea. Done.

[kochi, 2010-12-20 08:23:11]

LGTM

[arv (Not doing code reviews), 2010-12-20 19:03:36]

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos_accounts_options.html (right):

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_accounts_options.html:6: <img
src="warning.png">
Use CSS?

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos_accounts_options.js (right):

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_accounts_options.js:46:
$('ownerOnlyWarning')..classList.remove('hidden');
syntax error

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_accounts_options.js:48:
localStrings.getString('owner_user_id');
Can you move this to markup?

[satorux1, 2010-12-20 23:42:00]

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos_accounts_options.html (right):

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_accounts_options.html:6: <img
src="warning.png">
On 2010/12/20 19:03:36, arv wrote:
> Use CSS?

Just to make sure. Do you mean something like below?

<span id="warning"></span>

#warning {
  background-image: url('warning.png');
}

[arv (Not doing code reviews), 2010-12-20 23:57:09]

On Mon, Dec 20, 2010 at 15:42,  <satorux@chromium.org> wrote:
>
>
http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
> File chrome/browser/resources/options/chromeos_accounts_options.html
> (right):
>
>
http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
> chrome/browser/resources/options/chromeos_accounts_options.html:6: <img
> src="warning.png">
> On 2010/12/20 19:03:36, arv wrote:
>>
>> Use CSS?
>
> Just to make sure. Do you mean something like below?
>
> <span id="warning"></span>
>
> #warning {
>  background-image: url('warning.png');
> }
>

Yup

> http://codereview.chromium.org/5965003/
>

[satorux1, 2010-12-21 01:57:17]

Erik,

Thank you for the valuable comments!

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos_accounts_options.html (right):

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_accounts_options.html:6: <img
src="warning.png">
On 2010/12/20 23:42:00, satorux1 wrote:
> On 2010/12/20 19:03:36, arv wrote:
> > Use CSS?
> 
> Just to make sure. Do you mean something like below?
> 
> <span id="warning"></span>
> 
> #warning {
>   background-image: url('warning.png');
> }

Done.

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos_accounts_options.js (right):

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_accounts_options.js:46:
$('ownerOnlyWarning')..classList.remove('hidden');
On 2010/12/20 19:03:36, arv wrote:
> syntax error

Done.

http://codereview.chromium.org/5965003/diff/7001/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos_accounts_options.js:48:
localStrings.getString('owner_user_id');
On 2010/12/20 19:03:36, arv wrote:
> Can you move this to markup?

Done.

[arv (Not doing code reviews), 2010-12-21 18:57:25]

http://codereview.chromium.org/5965003/diff/16001/chrome/browser/resources/op...
File chrome/browser/resources/options/chromeos_accounts_options.html (right):

http://codereview.chromium.org/5965003/diff/16001/chrome/browser/resources/op...
chrome/browser/resources/options/chromeos_accounts_options.html:6: <span
id="warningIcon"></span>
Sorry I wasn't clear. I was suggesting putting the background image on the div
(ownerOnlyWarning). Changing from a img to a span only buys us the src
indirection. By moving it to the div we also reduce the dependency on the DOM
for layout of the warning

[satorux1, 2011-01-05 08:23:19]

Thank you for the comment. That makes a lot more sense.
http://codereview.chromium.org/5995011/ will fix this.

On 2010/12/21 18:57:25, arv wrote:
>
http://codereview.chromium.org/5965003/diff/16001/chrome/browser/resources/op...
> File chrome/browser/resources/options/chromeos_accounts_options.html (right):
> 
>
http://codereview.chromium.org/5965003/diff/16001/chrome/browser/resources/op...
> chrome/browser/resources/options/chromeos_accounts_options.html:6: <span
> id="warningIcon"></span>
> Sorry I wasn't clear. I was suggesting putting the background image on the div
> (ownerOnlyWarning). Changing from a img to a span only buys us the src
> indirection. By moving it to the div we also reduce the dependency on the DOM
> for layout of the warning