Last Comment Bug 454790 - Find does find text in -moz-user-select: none nodes, but doesn't highlight them
: Find does find text in -moz-user-select: none nodes, but doesn't highlight them
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: Selection (show other bugs)
: unspecified
: x86 Windows XP
: -- normal with 1 vote (vote)
: mozilla12
Assigned To: Josh Matthews [:jdm] (away until 9/3)
:
Mentors:
https://bugzilla.mozilla.org/attachme...
: 570713 693235 708047 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-11 06:37 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2016-02-17 07:21 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Allow programmatic selection of -moz-user-select: none frames. (1.26 KB, patch)
2012-01-12 08:22 PST, Josh Matthews [:jdm] (away until 9/3)
roc: review+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2008-09-11 06:37:14 PDT
The testcase is from bug 451252, when searching for the word 'mozilla', you'll notice that the first occurrence of that word in the page gets highlighted, but the second and third not, even though they are being found.
It seems to me the second and third occurrence should also get highlighted.
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2008-09-11 17:48:01 PDT
This probably more belongs in Core->Selection.

For me -moz-user-select: none; means, the text can't be selected by the user in any way (by mouse or by keyboard), but the text should still be selectable by dom methods or the find code.
Comment 2 Graeme McCutcheon [:graememcc] 2009-04-21 15:03:31 PDT
>For me -moz-user-select: none; means, the text can't be selected by the user in
>any way (by mouse or by keyboard), but the text should still be selectable by
>dom methods or the find code.

Well nsFrame::Handle{Press|Click|Drag} etc already have a IsSelectable check to disallow the user's selection there and then, so most of the time I think we're already only reaching nsFrame::SetSelected either when we are a user 
selection and the frame is selectable or when we are a programmatic selection.

The problem would be "Select All" - "cmd_selectAll" is used by the keypress handlers etc for commands coming from the UI, but equally it is also used for the same effect in other areas of the code too. Clearly we still need the IsSelectable check within SetSelected - it allows "Select All" logic to simply add a single range running from the document start to end, and let the painting logic worry about where the selection is painted - but for the above definition of what -user-select-none to work we would then need to execute the check conditionally on a UI select-all.

That in turn would mean SetSelected would need to be passed a parameter detailing whether the call was a result of a select all command that came from the UI, which I don't imagine is desirable.

roc, any thoughts?
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-04-21 15:18:45 PDT
Let's look at this another way: what is the best user experience when the user searches for text and there's a match in -moz-user-select:none?
1) Refusing to find the match will surprise the user. Bad.
2) Showing the text as a normal selection when the text cannot be selected manually is also surprising. Bad.
3) Showing the text with a different selection style may also be confusing. Bad.

Hmm. Actually I think the real usability problem here is -moz-user-select:none. Having special text that the user can't select in a document where all other text can be selected seems like it's just going to confuse people. Why do we need it?
Comment 4 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-04-21 19:08:08 PDT
I guess it's needed for xul documents, because it's used there to make text unselectable. Perhaps it is possible to make the css property only applicable to xul elements or for xul documents or for both?
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-04-21 19:12:43 PDT
Maybe we should just skip matches that are in, or partially in, -moz-user-select:none?
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-04-21 19:13:08 PDT
Not a great solution but not worse than all the others.
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-04-21 19:18:57 PDT
Perhaps mconnor could weigh in on the UX side of this problem. Mike, see comment #3.
Comment 8 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-04-22 01:40:02 PDT
(In reply to comment #5)
> Maybe we should just skip matches that are in, or partially in,
> -moz-user-select:none?

I don't agree with this. I (and I think every normal user) would feel cheated when you're searching for a word, which is in the document, but isn't highlighted just because it has -moz-user-select:none.
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-04-22 02:12:43 PDT
OK, suppose we go back to the idea that -moz-user-select:none only disables selection UI but content can be programmatically selected (my option 2 in comment #3). Suppose we have a -moz-user-select:none element in a document and the user starts a selection before the element and drags the end of the selection after the element. Should the element look selected or not?

I'm inclined to say it should, and select-all should select it too.
Comment 10 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-04-22 02:22:44 PDT
(In reply to comment #9)
> I'm inclined to say it should, and select-all should select it too.

I'm not sure I agree with you, but I don't think I care about it much either. It seems to me the whole -moz-user-select concept is kind of broken anyway.

Yeah, I think that content should be able to be programmatically selected, imho.
Comment 11 Graeme McCutcheon [:graememcc] 2009-04-22 10:28:28 PDT
So, it seems there's a not-dissimilar discussion taking place in m.d.platform

http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/c018736feaf7c65f
Comment 12 John P Baker 2010-06-09 02:02:15 PDT
*** Bug 570713 has been marked as a duplicate of this bug. ***
Comment 13 Alice0775 White 2011-10-09 21:36:47 PDT
*** Bug 693235 has been marked as a duplicate of this bug. ***
Comment 14 Mozilla RelEng Bot 2012-01-12 05:15:40 PST
Try run for 990f0f3158dc is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=990f0f3158dc
Results (out of 55 total builds):
    success: 48
    warnings: 7
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-990f0f3158dc
Comment 15 Martijn Wargers [:mwargers] (not working for Mozilla) 2012-01-12 05:22:01 PST
Mozilla RelEng Bot, what do you mean?
Comment 16 Josh Matthews [:jdm] (away until 9/3) 2012-01-12 08:22:33 PST
Created attachment 588039 [details] [diff] [review]
Allow programmatic selection of -moz-user-select: none frames.

I wrote a patch, but decided to see what tryserver thought of it before showing anybody else. Robert, I don't really understand all the complexities at play here, but this patch makes all nsTextFrames selectable if the caller is able to call SetSelectedRange. That means that mouse selection remains disallowed, because the input handling in nsFrame checks IsSelectable first.
Comment 17 Josh Matthews [:jdm] (away until 9/3) 2012-01-12 08:34:03 PST
Comment on attachment 588039 [details] [diff] [review]
Allow programmatic selection of -moz-user-select: none frames.

See previous comment.
Comment 18 Josh Matthews [:jdm] (away until 9/3) 2012-01-12 13:00:34 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/efdfbd08a433
Comment 19 Marco Bonardo [::mak] 2012-01-13 02:59:38 PST
https://hg.mozilla.org/mozilla-central/rev/efdfbd08a433
Comment 20 arni2033 2016-02-17 07:21:07 PST
*** Bug 708047 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.