Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2319303002:
Bug Fix: Title Case for Picker Window Title (Closed)
DescriptionBug Fix: Title Case for Picker Window Title
On Mac, desktop capture picker window's title is using sentence case, but by convention, it should be in title case.
This CL Fixes the bug.
BUG=644905
Committed: https://crrev.com/f8b6ed629f6adaa2bcb7969bc586f117994b137d
Cr-Commit-Position: refs/heads/master@{#417422}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Use if expression #
Total comments: 3
Patch Set 3 : Indent #Messages
Total messages: 22 (14 generated)
Description was changed from ========== Title Case for Picker Window Title BUG= ========== to ========== Title Case for Picker Window Title [Mac] BUG= ==========
Description was changed from ========== Title Case for Picker Window Title [Mac] BUG= ========== to ========== Bug Fix: Title Case for Picker Window Title [Mac] On Mac, desktop capture picker window's title is using sentence case, but by convention, it should be in title case. This CL Fixes the bug. BUG=644905 ==========
qiangchen@chromium.org changed reviewers: + msw@chromium.org
Description was changed from ========== Bug Fix: Title Case for Picker Window Title [Mac] On Mac, desktop capture picker window's title is using sentence case, but by convention, it should be in title case. This CL Fixes the bug. BUG=644905 ========== to ========== Bug Fix: Title Case for Picker Window Title On Mac, desktop capture picker window's title is using sentence case, but by convention, it should be in title case. This CL Fixes the bug. BUG=644905 ==========
qiangchen@chromium.org changed reviewers: + ainslie@chromium.org
Can you take a look? By convention, we need Title Case for Window Title.
https://codereview.chromium.org/2319303002/diff/1/chrome/app/generated_resour... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2319303002/diff/1/chrome/app/generated_resour... chrome/app/generated_resources.grd:13912: + <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE_MAC" desc="Title for the window picker dialog shown when desktop capture is requested by an app."> Follow the <if expr="use_titlecase"> pattern of other similar strings.
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by qiangchen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Fixed. But I have a question about <if expr="use_titlecase"> Thanks. https://codereview.chromium.org/2319303002/diff/1/chrome/app/generated_resour... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2319303002/diff/1/chrome/app/generated_resour... chrome/app/generated_resources.grd:13912: + <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE_MAC" desc="Title for the window picker dialog shown when desktop capture is requested by an app."> On 2016/09/07 22:39:22, msw wrote: > Follow the <if expr="use_titlecase"> pattern of other similar strings. Tested, and I found this way works. But I am curious, how is the string selection determined? Namely, How does the system know I need "Share Your Screen", not "Share your screen" as the window title.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with indentation nits https://codereview.chromium.org/2319303002/diff/1/chrome/app/generated_resour... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2319303002/diff/1/chrome/app/generated_resour... chrome/app/generated_resources.grd:13912: + <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE_MAC" desc="Title for the window picker dialog shown when desktop capture is requested by an app."> On 2016/09/08 16:46:03, qiangchenC wrote: > On 2016/09/07 22:39:22, msw wrote: > > Follow the <if expr="use_titlecase"> pattern of other similar strings. > > Tested, and I found this way works. > > But I am curious, how is the string selection determined? Namely, How does the > system know I need "Share Your Screen", not "Share your screen" as the window > title. Each platform sets the use_titlecase build flag value, and that determines which string is included in the string resource pak files built/shipped/used. Take a look at use_titlecase mentions build/common.gypi, tools/grit/grit_rule.gni, etc. if you're interested in the nitty-gritty. https://codereview.chromium.org/2319303002/diff/40001/chrome/app/generated_re... File chrome/app/generated_resources.grd (left): https://codereview.chromium.org/2319303002/diff/40001/chrome/app/generated_re... chrome/app/generated_resources.grd:13909: <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE" desc="Title for the window picker dialog shown when desktop capture is requested by an app."> nit: fix indentation https://codereview.chromium.org/2319303002/diff/40001/chrome/app/generated_re... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2319303002/diff/40001/chrome/app/generated_re... chrome/app/generated_resources.grd:13915: + <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE" desc="Title for the window picker dialog shown when desktop capture is requested by an app."> ditto nit: fix indentation https://codereview.chromium.org/2319303002/diff/40001/chrome/app/generated_re... chrome/app/generated_resources.grd:13916: + Share Your Screen nit: this should be indented by two spaces from the <message> tag, not four.
https://codereview.chromium.org/2319303002/diff/1/chrome/app/generated_resour... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2319303002/diff/1/chrome/app/generated_resour... chrome/app/generated_resources.grd:13912: + <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE_MAC" desc="Title for the window picker dialog shown when desktop capture is requested by an app."> On 2016/09/08 20:14:10, msw wrote: > On 2016/09/08 16:46:03, qiangchenC wrote: > > On 2016/09/07 22:39:22, msw wrote: > > > Follow the <if expr="use_titlecase"> pattern of other similar strings. > > > > Tested, and I found this way works. > > > > But I am curious, how is the string selection determined? Namely, How does the > > system know I need "Share Your Screen", not "Share your screen" as the window > > title. > > Each platform sets the use_titlecase build flag value, and that determines which > string is included in the string resource pak files built/shipped/used. Take a > look at use_titlecase mentions build/common.gypi, tools/grit/grit_rule.gni, etc. > if you're interested in the nitty-gritty. Acknowledged.
The CQ bit was checked by qiangchen@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org Link to the patchset: https://codereview.chromium.org/2319303002/#ps60001 (title: "Indent")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Bug Fix: Title Case for Picker Window Title On Mac, desktop capture picker window's title is using sentence case, but by convention, it should be in title case. This CL Fixes the bug. BUG=644905 ========== to ========== Bug Fix: Title Case for Picker Window Title On Mac, desktop capture picker window's title is using sentence case, but by convention, it should be in title case. This CL Fixes the bug. BUG=644905 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Bug Fix: Title Case for Picker Window Title On Mac, desktop capture picker window's title is using sentence case, but by convention, it should be in title case. This CL Fixes the bug. BUG=644905 ========== to ========== Bug Fix: Title Case for Picker Window Title On Mac, desktop capture picker window's title is using sentence case, but by convention, it should be in title case. This CL Fixes the bug. BUG=644905 Committed: https://crrev.com/f8b6ed629f6adaa2bcb7969bc586f117994b137d Cr-Commit-Position: refs/heads/master@{#417422} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/f8b6ed629f6adaa2bcb7969bc586f117994b137d Cr-Commit-Position: refs/heads/master@{#417422} |
