Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2534733002:
Simplify print preview system dialog message (Closed)
DescriptionSimplify print preview system dialog message
print_preview_ui.cc previously specified different specified a system
print dialog shortcut message of "(Ctrl+Shift+P)" for Windows and
ChromeOS, and "(Shift+Ctrl+P)" for everything else except Mac OSX. These
have been unified such that everything besides Mac OSX uses
"(Ctrl+Shift+P)".
Committed: https://crrev.com/76a57be227d33f9a5113ffb80689df34c349c35a
Cr-Commit-Position: refs/heads/master@{#434915}
Patch Set 1 #
Messages
Total messages: 18 (9 generated)
The CQ bit was checked by gozzard@google.com 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...
gozzard@google.com changed reviewers: + nainar@chromium.org, thestig@chromium.org
Hi, PTAL Cheers
thestig, This was sent more to get Gozz here familiar with the code review system. It's an lgtm on my end. Please feel free to dismiss if there is a reason why we maintain two separate keyboard shortcuts.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/11/28 02:50:56, nainar wrote: > thestig, > > This was sent more to get Gozz here familiar with the code review system. > > It's an lgtm on my end. Please feel free to dismiss if there is a reason why we > maintain two separate keyboard shortcuts. This came about from https://codereview.chromium.org/7721001 I launched gedit on my computer and it shows shift+ctrl+$X for some value of $X. This is what the original code was based on. OTOH, KDE apps, LibreOffice, and Firefox all show ctrl+shift+$X. Also, in Chrome's wrench, err, hot dog, err, 3 dots menu, other shortcuts are shown as ctrl+shift+$X. So that's a vote do go with this CL.
thestig@chromium.org changed reviewers: + thomasanderson@chromium.org
+thomasanderson for his opinion and to help sort out this shift+ctrl business. ^
On 2016/11/29 02:19:12, Lei Zhang (OOO) wrote: > +thomasanderson for his opinion and to help sort out this shift+ctrl business. ^ It's easier to change this instance to ctrl+shift+$X rather than change all other parts of chrome to say shift+ctrl+$X, so this lgtm thestig@ I thought you were supposed to be on vacation!
lgtm 3 On 2016/11/29 02:28:52, Tom Anderson wrote: > It's easier to change this instance to ctrl+shift+$X rather than change all > other parts of chrome to say shift+ctrl+$X, so this lgtm Sounds like we should file a bug for going with ctrl+shift+$X. > thestig@ I thought you were supposed to be on vacation! As you can see, I'm not very good at it. I need to practice more.
The CQ bit was checked by gozzard@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1480398209417520, "parent_rev":
"615a0960a49bedfa4a3000610cf4bcd6c9903af0", "commit_rev":
"046203e17617808b5de14e0ac244657e6952cd1d"}
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Simplify print preview system dialog message print_preview_ui.cc previously specified different specified a system print dialog shortcut message of "(Ctrl+Shift+P)" for Windows and ChromeOS, and "(Shift+Ctrl+P)" for everything else except Mac OSX. These have been unified such that everything besides Mac OSX uses "(Ctrl+Shift+P)". ========== to ========== Simplify print preview system dialog message print_preview_ui.cc previously specified different specified a system print dialog shortcut message of "(Ctrl+Shift+P)" for Windows and ChromeOS, and "(Shift+Ctrl+P)" for everything else except Mac OSX. These have been unified such that everything besides Mac OSX uses "(Ctrl+Shift+P)". Committed: https://crrev.com/76a57be227d33f9a5113ffb80689df34c349c35a Cr-Commit-Position: refs/heads/master@{#434915} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/76a57be227d33f9a5113ffb80689df34c349c35a Cr-Commit-Position: refs/heads/master@{#434915} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
