New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 865199 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Do not display emoji menu in Password fields

Project Member Reported by yyushkina@chromium.org, Jul 18

Issue description

Right now we show the Emoji option in the context menu on Password fields. We shouldn't, the same way we don't on Mac and Windows.
 
Screen Shot 2018-07-18 at 2.00.13 PM.png
3.8 MB View Download
Thanks for the report. Can I ask what version of Canary are you running for Mac? On my Mac Canary (69.0.3492.0), "Emoji & Symbols" shows up for password fields.
Cc: ramyan@chromium.org
I was using stable (M67). And you're right - it's showing up in M69 Mac Canary. I'm guessing this is because we don't have the same logic in Views (since M69 has MacViews and M67 doesn't). 
Maybe emoji in passwords are not a good idea, but should we prevent users from trying?
This is a problem with other input types too. As discussed elsewhere, let's exclude certain input types (and keep emoji on password fields).
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1b8ac5ab4a18310980b7be1bfb3f6d1a0cd2baa

commit b1b8ac5ab4a18310980b7be1bfb3f6d1a0cd2baa
Author: Darren Shen <shend@chromium.org>
Date: Sun Jul 22 23:54:16 2018

[VK] Don't show emoji menu item in context menu for certain input types.

We shouldn't show the emoji menu item in the render view context menu
for "number", "tel" and "other" (dates, time, checkboxes etc.) input
types.

We deliberately implement this as a blacklist (don't show for these
input types) instead of a whitelist (only show for these input types),
since not being able to enter emoji when you should be able to is much
more frustrating than having an extraneous emoji item.

Note that this permits emoji on "password" fields, but apparently that
could be a thing.

Bug:  865199 
Change-Id: I7b2b977f435cee0c4f98c357187b45d90a08ecfc
Reviewed-on: https://chromium-review.googlesource.com/1142851
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577098}
[modify] https://crrev.com/b1b8ac5ab4a18310980b7be1bfb3f6d1a0cd2baa/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/b1b8ac5ab4a18310980b7be1bfb3f6d1a0cd2baa/third_party/blink/public/web/web_context_menu_data.h
[modify] https://crrev.com/b1b8ac5ab4a18310980b7be1bfb3f6d1a0cd2baa/third_party/blink/renderer/core/page/context_menu_controller.cc

Labels: Merge-Request-69
Project Member

Comment 8 by sheriffbot@chromium.org, Jul 24

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 24

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02a66a109dc5b18b5c9f80a41a4f44ebccf9222e

commit 02a66a109dc5b18b5c9f80a41a4f44ebccf9222e
Author: Darren Shen <shend@chromium.org>
Date: Tue Jul 24 01:23:55 2018

[Merge M69] Don't show emoji menu item in context menu for certain input types.

We shouldn't show the emoji menu item in the render view context menu
for "number", "tel" and "other" (dates, time, checkboxes etc.) input
types.

We deliberately implement this as a blacklist (don't show for these
input types) instead of a whitelist (only show for these input types),
since not being able to enter emoji when you should be able to is much
more frustrating than having an extraneous emoji item.

Note that this permits emoji on "password" fields, but apparently that
could be a thing.

TBR=ramyan@chromium.org

Bug:  865199 
Change-Id: I7b2b977f435cee0c4f98c357187b45d90a08ecfc
Reviewed-on: https://chromium-review.googlesource.com/1142851
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577098}(cherry picked from commit b1b8ac5ab4a18310980b7be1bfb3f6d1a0cd2baa)
Reviewed-on: https://chromium-review.googlesource.com/1147820
Cr-Commit-Position: refs/branch-heads/3497@{#33}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/02a66a109dc5b18b5c9f80a41a4f44ebccf9222e/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/02a66a109dc5b18b5c9f80a41a4f44ebccf9222e/third_party/blink/public/web/web_context_menu_data.h
[modify] https://crrev.com/02a66a109dc5b18b5c9f80a41a4f44ebccf9222e/third_party/blink/renderer/core/page/context_menu_controller.cc

Status: Fixed (was: Assigned)

Sign in to add a comment