Text in "alert" boxes not selectable
Reported by
sie...@boerse-go.de,
May 15 2017
|
|||||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36
Steps to reproduce the problem:
1. alert('some Text')
2. try to select "some Text" in the dialog box
3. => not working
What is the expected behavior?
Text should be selectable
What went wrong?
Text is NOT selectable
Did this work before? N/A
Chrome version: 58.0.3029.110 Channel: stable
OS Version: OS X 10.12.4
Flash Version:
probably a regression:
https://bugs.chromium.org/p/chromium/issues/detail?id=5879
but this ticket is closed according to last comment
,
May 16 2017
Observations: ------------ 1.Able to reproduce the issue on mac 10.12.4 with Fresh chrome (58.0.3029.110) installation ( Uninstall existing chrome & install new chrome build) 2.Unable to reproduce the issue on mac 10.12.4 using chrome reported version-58.0.3029.110 when chrome is over installed on old version. 3.Issue working as intended from M30 builds to latest stable only when user over installed chrome. Note: Unable to reproduce the issue on Windows 7 & ubuntu 14.04 using chrome reported version-58.0.3029.110. Removing needs-bisect label & marking it as 'untriaged' to get more inputs from dev. Thank you..!!
,
May 16 2017
Issue 719978 has been merged into this issue.
,
May 16 2017
,
May 16 2017
[chrome mac triage] Assigning this ellyjones@ since she has been working on dialogs.
,
Oct 11 2017
I had a look at fixing this for MacViews. It seems that, while the label is configured to be selectable, it never actually receives OnMouseDown/OnMouseDragged (?). I tried in views_examples_with_content_exe as well, and it appears that Views labels do not support text selection. I remember karandeepb@ working on this as part of some earlier MacViews work but I'm not sure the state of it. tapted@, do you remember what was going on with that? For Cocoa, the fix is not too hard: <https://chromium-review.googlesource.com/c/chromium/src/+/714036>
,
Oct 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e9d5e38c448579d1b61ce10fd9dd204ddecb6148 commit e9d5e38c448579d1b61ce10fd9dd204ddecb6148 Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Wed Oct 11 21:05:01 2017 cocoa: make javascript dialog text selectable This change: 1) Makes javascript dialog title and message selectable; 2) Moves the attributes of the title and message onto the NSTextField{,Cell} as appropriate instead of the NSAttributedString, because otherwise selecting the text reverts the string attributes (!). Bug: 722217 Change-Id: I281187e2b1c14e80d0b346401b7caf41b1cc4326 Reviewed-on: https://chromium-review.googlesource.com/714036 Reviewed-by: Sidney San Martín <sdy@chromium.org> Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#508112} [modify] https://crrev.com/e9d5e38c448579d1b61ce10fd9dd204ddecb6148/chrome/browser/ui/cocoa/constrained_window/constrained_window_alert.mm
,
Oct 11 2017
The Cocoa side of this is fixed. I'm kicking the Views side over to tapted@.
,
Oct 11 2017
We need to use RenderTextHarfbuzz in the Label.
changing
void Label::Init(const base::string16& text, const gfx::FontList& font_list) {
render_text_.reset(gfx::RenderText::CreateInstance());
to
void Label::Init(const base::string16& text, const gfx::FontList& font_list) {
render_text_.reset(gfx::RenderText::CreateInstanceForEditing());
should fix this.. We may even be able to do that in SetSelectable..
,
Oct 12 2017
Verified the fix on Mac 10.12.6 using latest chrome version #63.0.3238.0 as per the comment #0. Attaching screen cast for reference. Observed that "some Text" in the dialog box got selected as expected. Hence, the fix is working as expected. Adding the verified labels. Thanks...!!
,
Oct 12 2017
Got it - thanks tapted@ :) I'll have a go at implementing this.
,
Oct 12 2017
Ooh, note we are _probably_ not quite ready to go all-in on
Label::Init(const base::string16& text, const gfx::FontList& font_list) {
render_text_.reset(gfx::RenderText::CreateInstanceForEditing());
There are some emoji I want to fix.
but something like that in SetSelectable(), guarded by #if MAC, and followed by a call to Label::CreateRenderText() or similar, is probably OK.
,
Nov 29 2017
tapted: what's the situation with the emoji? and/or is there a bug for that? I'd like to make the Views side of this fix if we can.
,
Nov 29 2017
Emoji were hopefully fixed in Issue 739641 . This should be good to go forward. There are performance concerns - Issue 785522 - but they should be fixed for m65, and probably are not noticeable when there are fewer than ~100 lines of text.
,
Dec 1 2017
Elly: heads up - I'm planning just to make views::Label always use RenderTextHarfBuzz on Mac. Multiline labels are currently super-slow because RenderTextMac does not support multiline, so views::Label can not cache its size calculations. See Issue 789118 . RTM is about 10x slower than RTHB (100ms vs 10ms). I explored a simple caching fix for RTM, but I don't think it's worth the effort.
,
Dec 1 2017
Cool - is there a CL up for that?
,
Dec 4 2017
,
Dec 4 2017
> Cool - is there a CL up for that? https://chromium-review.googlesource.com/c/chromium/src/+/805534 ('ware dragons)
,
Dec 8 2017
That CL landed as r522617, but it may have stirred up a spiders nest. Hopefully it will stick. In other news, that change also simply causes this issue to be fixed \o/.
,
Dec 8 2017
In that case, I'm assigning this to you :)
,
Jan 30 2018
That CL landed. This is fixed with chrome://flags/#secondary-ui-md enabled , target: ~m66. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by nyerramilli@google.com
, May 15 2017