Issue metadata
Sign in to add a comment
|
mushrome: Stylus icon shows in status area on non-stylus device (link) |
||||||||||||||||||||||||
Issue descriptionchrome --mus on link, Chrome r468701, self-built test image 60.0.3088.0 The TrayPalette system tray icon appears. The menu spawns when I click on it. This is weird, since this isn't a stylus device. The icon does not show up with normal chrome or with chrome --mash. This does not repro on desktop. Are we installing an incorrect delegate here? Is there some chromeos::GetAshConfig() check that needs to be fixed?
,
May 2 2017
Duplicated with https://bugs.chromium.org/p/chromium/issues/detail?id=712887
,
May 2 2017
This bug might be slightly different, the bug (in 712887) will add the palette but no show it until press the accelerator as in normal chrome. I will look into why --mus has this different behavior.
,
May 3 2017
To run --mus you need to edit /etc/chrome_dev.conf to add the --mus flag. How I do this: stop ui cp /etc/chrome_dev.conf /home/ # edit file mount --bind /home/chrome_dev.conf /etc/chrome_dev.conf start ui
,
May 3 2017
When use the --mus at link, it returns true that the InputDeviceManager has stylus: https://cs.chromium.org/chromium/src/ash/system/palette/palette_utils.cc?type=cs&l=34
,
May 4 2017
I found out the reason is that when TouchscreenDevice() is called, the is_stylus value is not explicitly init to false: https://cs.chromium.org/chromium/src/ui/events/devices/touchscreen_device.cc?dr=C&l=13 Add "is_stylus(false)" to the init list, will fix this bug.
,
May 4 2017
sky@ and jamescook@, I updated the cl[1] to fix this bug as well, please review. Please let me know if I should open another cl for fixing this bug only so that I can land the original cl[1] without delay too long. Thanks. [1] https://codereview.chromium.org/2825383003/
,
May 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313 commit 53e9c2e81bf4fe7d0f6bcaebca0c717b19430313 Author: wutao <wutao@chromium.org> Date: Thu May 04 19:19:29 2017 Fix stylus tools palette. HandleShowStylusTools crashes at desktop ChromeOS UI and shows stylus tools on device does not have stylus. Adding more checks when we should HandleShowStylusTools and when to AddPaletteTray. BUG= 712887 , 717674 , 717422 TEST=Manual && AboutFlagsHistogramTest.CheckHistograms Review-Url: https://codereview.chromium.org/2825383003 Cr-Commit-Position: refs/heads/master@{#469418} [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/accelerators/accelerator_controller.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/ash_switches.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/ash_switches.h [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/system/palette/palette_tray.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/system/palette/palette_tray.h [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/system/palette/palette_utils.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/system/palette/palette_utils.h [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/system/status_area_widget.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ash/system/status_area_widget_unittest.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/chrome/browser/about_flags.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/chrome/browser/chromeos/login/chrome_restart_request.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/chrome/browser/chromeos/note_taking_helper_unittest.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/tools/metrics/histograms/enums.xml [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ui/events/devices/input_device.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ui/events/devices/touchscreen_device.cc [modify] https://crrev.com/53e9c2e81bf4fe7d0f6bcaebca0c717b19430313/ui/events/devices/touchscreen_device.h
,
May 16 2017
Re assign this to rjkroege@ I fixed part of the problem so the stylus palette will not show in mus mode in LINK, to set the is_stylus(false) in TouchscreenDevice() [1] [1] https://cs.chromium.org/chromium/src/ui/events/devices/touchscreen_device.cc?dr=C&l=13 But there is another problem. To show the stylus palette, the device type must also be ui::InputDeviceType::INPUT_DEVICE_INTERNAL (enum, 0) [2] [2] https://cs.chromium.org/chromium/src/ash/system/palette/palette_utils.cc?type=cs&l=36 I am not familiar with the flow of code in mus, but I am afraid the device type is not set to ui::InputDeviceType::INPUT_DEVICE_UNKNOWN (enum, 2) in the input_devices service in mus [3] and set is_stylus to false in [4]. InputDevice::InputDevice(), the type() default is 0? Which means it is internal device. [3] https://cs.chromium.org/chromium/src/out/Debug/gen/ui/events/devices/mojo/input_devices.mojom.cc?dr=C&l=33 and TouchscreenDevice::TouchscreenDevice() [4] https://cs.chromium.org/chromium/src/out/Debug/gen/ui/events/devices/mojo/input_devices.mojom.cc?dr=C&l=62
,
May 16 2017
Also, bool is_stylus is missing in the struct TouchscreenDevice in input_devices.mojom: https://cs.chromium.org/chromium/src/ui/events/devices/mojo/input_devices.mojom?type=cs&l=30 It seems hard to keep them sync, if someone adds new members in input_device or touchscreen_device.
,
Aug 22 2017
sky@ can you find someone else to work on this?
,
Aug 22 2017
This no longer repros on tip of tree.
,
Nov 2 2017
,
Feb 26 2018
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by jdufault@chromium.org
, May 2 2017Status: Assigned (was: Untriaged)