NTP: RTL characters anywhere in a shortcut name displays the whole name in RTL paragraph |
|||||||||||||||||||
Issue descriptionChrome Version: 72.0.3626.0 (Developer Build) (64-bit), also repros on 70.0.3538.110 OS: tested on Linux and MacOS What steps will reproduce the problem? (1) On the new tab page, click "Add shortcut" and add a URL with RTL into the URL field, click Done A few things happen: (1) Name in the edit shortcut prompt is displayed how it would be displayed in the omnibox (2) URL field falls back to punycode (3) On the page the URL displays backwards/out of order (4) Default icon letter matches first letter of hostname in punycode Desired behavior should probably be: (1) Default Name and URL field matches how displayed in omnibox (2) Display on the page should match Name field (3) Default icon can maybe stay as is with first punycode letter?
,
Nov 30
I think this might be due to https://cs.chromium.org/chromium/src/components/ntp_tiles/most_visited_sites.cc?l=850 where GURL::host() is called, although I'm not very familiar with how the NTP icons work.
,
Nov 30
This has nothing to do with URL parsing or display. When creating a shortcut, you have a "Name" and "URL" field, and this is entirely to do with the display of the Name field (which you happen to be typing a URL string into). This bug is more general than URL strings: it seems that what this page is doing is if it finds any RTL character in the string, it renders the string in an RTL paragraph. For example, if you set the name to "test مثال string", it renders as: test مثال string This is technically valid (who's to say what the default paragraph direction should be), but quite unusual. We usually either set the default paragraph direction to either: a) the UI direction (based on the user's language), or b) the direction of the first strong character (in this case, LTR). I don't know enough about WebUI to see how this is happening, but that's what the issue is and what the fix should be. This has nothing to do with URL rendering.
,
Nov 30
Kristi - since you've already looked at RTL for this feature, do you have additional thoughts here?
,
Nov 30
NTP is not WebUI, so RTL direction for title text is determined here: https://cs.chromium.org/chromium/src/chrome/renderer/searchbox/searchbox_extension.cc?dr=CSs&g=0&l=153 I assume this was fine pre-M69 since URLs were rarely (if ever) set as the title for history entries, which was where the Most Visited tiles are taken from. (b) sounds like a reasonable fix for this.
,
Dec 1
,
Dec 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64103b173069261a6986c13f6cd32658153cdd39 commit 64103b173069261a6986c13f6cd32658153cdd39 Author: Kristi Park <kristipark@chromium.org> Date: Mon Dec 03 22:12:45 2018 [NTP] Determine shortcut title direction by the first strong character and adjust RTL for edit shortcut dialog fields Determine text direction using the first strong character instead of whether the text contains any RTL character. Also add direction support in the edit shortcut dialog fields LTR browser: https://screenshot.googleplex.com/11Z58bTugPV.png https://screenshot.googleplex.com/ne61RSr24qg.png https://screenshot.googleplex.com/fr28i5Zzceh.png RTL browser: https://screenshot.googleplex.com/ZEGVSdiOsTS.png https://screenshot.googleplex.com/cNtF8s6htp9.png https://screenshot.googleplex.com/RHoZu97GL8y.png Bug: 910385 Change-Id: I842c1479e3a3003e218d58d5ccfa511fdddc4963 Reviewed-on: https://chromium-review.googlesource.com/c/1357637 Commit-Queue: Kristi Park <kristipark@chromium.org> Reviewed-by: Ramya Nagarajan <ramyan@chromium.org> Cr-Commit-Position: refs/heads/master@{#613283} [modify] https://crrev.com/64103b173069261a6986c13f6cd32658153cdd39/chrome/browser/resources/local_ntp/custom_links_edit.css [modify] https://crrev.com/64103b173069261a6986c13f6cd32658153cdd39/chrome/browser/resources/local_ntp/custom_links_edit.js [modify] https://crrev.com/64103b173069261a6986c13f6cd32658153cdd39/chrome/renderer/searchbox/searchbox_extension.cc
,
Dec 3
,
Dec 4
Tried testing the issue on reported chrome version #72.0.3626.0 and latest chrome version #73.0.3630.0 using Windows 10, Ubuntu 17.10 and Mac OS 10.14 by following steps as per comment#0. Below are the observations noted while testing the issue. Observations: ============= 1.In chrome version #72.0.3626.0, when saved a shorcut it got saved as shown in the "Different URLs with RTL text.png ". 2.In chrome version #73.0.3630.0, when saved a shortcut it got saved in the same format as of how it gets displayed in omnibox. 3.Unable to see any difference in the edit shortcut prompt. Attached screenshots for reference. @Kristi Park: Could you please review attached screenshots and help us in confirming the fix, let us know if anything is being missed from our end. Thanks.!
,
Dec 4
|
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by livvielin@google.com
, Nov 29