Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2758443003:
Remove usb option from manual printer configuration. (Closed)
|
Created:
3 years, 9 months ago by skau Modified:
3 years, 9 months ago CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org, arv+watch_chromium.org, Carlson, weifangsun Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove usb option from manual printer configuration.
It's simply too hard to ask a user to configure a USB printer manually.
Remove to option to prevent a terrible experience. The USB detector
committed soon.
BUG=699285
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2758443003
Cr-Commit-Position: refs/heads/master@{#458192}
Committed: https://chromium.googlesource.com/chromium/src/+/f7f370dba5703b1df78ae7a6bc54b498fcd18589
Patch Set 1 #
Total comments: 3
Messages
Total messages: 25 (11 generated)
Description was changed from ========== Remove usb option from manual printer configuration. It's simply too hard to ask a user to configure a USB printer manually. Remove to option to prevent a terrible experience. The USB detector committed soon. BUG= ========== to ========== Remove usb option from manual printer configuration. It's simply too hard to ask a user to configure a USB printer manually. Remove to option to prevent a terrible experience. The USB detector committed soon. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
skau@chromium.org changed reviewers: + stevenjb@chromium.org
stevenjb@: I believe you're the relevant OWNER for this package. The rest: FYI
xdai@chromium.org changed reviewers: + xdai@chromium.org
https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html (left): https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:176: <option value="usb">$i18n{printerProtocolUsb}</option> drive-by comment: I think you should also remove IDS_SETTINGS_PRINTING_CUPS_PRINTER_PROTOCOL_USB and the corresponding string from settings_strings.grdp?
We need an issue to track this (and please cc the appropriate PM). https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html (left): https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:176: <option value="usb">$i18n{printerProtocolUsb}</option> On 2017/03/17 03:39:49, xdai1 wrote: > drive-by comment: I think you should also remove > IDS_SETTINGS_PRINTING_CUPS_PRINTER_PROTOCOL_USB and the corresponding string > from settings_strings.grdp? That would be correct.
On 2017/03/17 16:55:11, stevenjb wrote: > We need an issue to track this (and please cc the appropriate PM). > > https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... > File > chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html > (left): > > https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... > chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:176: > <option value="usb">$i18n{printerProtocolUsb}</option> > On 2017/03/17 03:39:49, xdai1 wrote: > > drive-by comment: I think you should also remove > > IDS_SETTINGS_PRINTING_CUPS_PRINTER_PROTOCOL_USB and the corresponding string > > from settings_strings.grdp? > > That would be correct. Also, do we still want to remove this with https://codereview.chromium.org/2738323003/ pending? It's a little unclear.
Description was changed from ========== Remove usb option from manual printer configuration. It's simply too hard to ask a user to configure a USB printer manually. Remove to option to prevent a terrible experience. The USB detector committed soon. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Remove usb option from manual printer configuration. It's simply too hard to ask a user to configure a USB printer manually. Remove to option to prevent a terrible experience. The USB detector committed soon. BUG=699285 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
On 2017/03/17 16:58:48, stevenjb wrote: > On 2017/03/17 16:55:11, stevenjb wrote: > > We need an issue to track this (and please cc the appropriate PM). > > > > > https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... > > File > > chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html > > (left): > > > > > https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... > > > chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:176: > > <option value="usb">$i18n{printerProtocolUsb}</option> > > On 2017/03/17 03:39:49, xdai1 wrote: > > > drive-by comment: I think you should also remove > > > IDS_SETTINGS_PRINTING_CUPS_PRINTER_PROTOCOL_USB and the corresponding string > > > from settings_strings.grdp? > > > > That would be correct. > > Also, do we still want to remove this with > https://codereview.chromium.org/2738323003/ pending? It's a little unclear. To clarify, we're not removing the support for USB printers. We're just removing the ability to configure it through the manual configuration flow. The referenced CL obsoletes the current approach.
The PM is aware and I've added the appropriate bug. My apologies for the omission. Other comments have been replied to. +weifangsun@: PM FYI https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html (left): https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:176: <option value="usb">$i18n{printerProtocolUsb}</option> On 2017/03/17 16:55:11, stevenjb wrote: > On 2017/03/17 03:39:49, xdai1 wrote: > > drive-by comment: I think you should also remove > > IDS_SETTINGS_PRINTING_CUPS_PRINTER_PROTOCOL_USB and the corresponding string > > from settings_strings.grdp? > > That would be correct. No, it's still used on the printer detail page. https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/printi... We can still setup USB printers. It's just that setting up USB printers in this flow doesn't make a lot of sense anymore.
OK, lgtm, thanks!
On 2017/03/17 19:52:40, skau wrote: > The PM is aware and I've added the appropriate bug. My apologies for the > omission. Other comments have been replied to. > > +weifangsun@: PM FYI > > https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... > File > chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html > (left): > > https://codereview.chromium.org/2758443003/diff/1/chrome/browser/resources/se... > chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:176: > <option value="usb">$i18n{printerProtocolUsb}</option> > On 2017/03/17 16:55:11, stevenjb wrote: > > On 2017/03/17 03:39:49, xdai1 wrote: > > > drive-by comment: I think you should also remove > > > IDS_SETTINGS_PRINTING_CUPS_PRINTER_PROTOCOL_USB and the corresponding string > > > from settings_strings.grdp? > > > > That would be correct. > > No, it's still used on the printer detail page. > https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/printi... > > We can still setup USB printers. It's just that setting up USB printers in this > flow doesn't make a lot of sense anymore. oh sorry! Didn't notice that we still need it to show USB printer's detail info. lgtm.
The CQ bit was checked by skau@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by skau@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by skau@chromium.org
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": 1490040864311760, "parent_rev":
"9c08889d47139672b311b30e9075a57459a6ec86", "commit_rev":
"f7f370dba5703b1df78ae7a6bc54b498fcd18589"}
Message was sent while issue was closed.
Description was changed from ========== Remove usb option from manual printer configuration. It's simply too hard to ask a user to configure a USB printer manually. Remove to option to prevent a terrible experience. The USB detector committed soon. BUG=699285 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Remove usb option from manual printer configuration. It's simply too hard to ask a user to configure a USB printer manually. Remove to option to prevent a terrible experience. The USB detector committed soon. BUG=699285 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2758443003 Cr-Commit-Position: refs/heads/master@{#458192} Committed: https://chromium.googlesource.com/chromium/src/+/f7f370dba5703b1df78ae7a6bc54... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/f7f370dba5703b1df78ae7a6bc54... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
