Closed Bug 454790 Opened 16 years ago Closed 12 years ago

Find does find text in -moz-user-select: none nodes, but doesn't highlight them

Categories

(Core :: DOM: Selection, defect)

Product:
Core
Component:
DOM: Selection
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla12

People

(Reporter: martijn.martijn, Assigned: jdm)

References

(
URL
)

Details

(Keywords: testcase)

Attachments

(1 file)

Allow programmatic selection of -moz-user-select: none frames.
1.26 KB, patch
roc
: review+
Details | Diff | Splinter Review 
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.
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.
Component: Find Backend → Selection
QA Contact: find-backend → 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.

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?
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?
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?
Maybe we should just skip matches that are in, or partially in, -moz-user-select:none?
Not a great solution but not worse than all the others.
Perhaps mconnor could weigh in on the UX side of this problem. Mike, see comment #3.
(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.
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.
(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.
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
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
Mozilla RelEng Bot, what do you mean?
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 on attachment 588039 [details] [diff] [review]
Allow programmatic selection of -moz-user-select: none frames.

See previous comment.
Attachment #588039 - Flags: feedback?(roc)
https://hg.mozilla.org/mozilla-central/rev/efdfbd08a433
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: