WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
52195
REGRESSION(
r71934
) Can't type in search edit field on skin-one.com
https://bugs.webkit.org/show_bug.cgi?id=52195
Summary
REGRESSION(r71934) Can't type in search edit field on skin-one.com
Adele Peterson
Reported
2011-01-10 21:51:19 PST
<
rdar://problem/8807518
> It looks like this site is trying to prevent copying of resources by disabling selection: document.onselectstart=new Function ("return false") You can reproduce the problem with this markup: <script>document.onselectstart=new Function("return false")</script> <input> Or with this url: data:text/html,%3Cscript%3Edocument.onselectstart=new%20Function%20(%22return%20false%22)%3C/script%3E%3Cinput%3E I think before this change, that wouldn't have prevented setting selection in a text field. Firefox doesn't appear to allow preventing selection start from this event at all. We should see what IE does. 
Attachments
Patch
(16.21 KB, patch)
2011-01-11 11:10 PST
,
Dimitri Glazkov (Google)
no flags
Details
Formatted Diff
Diff
With better ChangeLog descriptions.
(16.85 KB, patch)
2011-01-11 11:13 PST
,
Dimitri Glazkov (Google)
eric
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Dimitri Glazkov (Google)
Comment 1
2011-01-11 07:42:29 PST
My booger. Will fix.
Dimitri Glazkov (Google)
Comment 2
2011-01-11 07:42:54 PST
(In reply to
comment #0
)
> <
rdar://problem/8807518
> > > It looks like this site is trying to prevent copying of resources by disabling selection: > > document.onselectstart=new Function ("return false") > > You can reproduce the problem with this markup: > <script>document.onselectstart=new Function("return false")</script> > <input> > > Or with this url: > data:text/html,%3Cscript%3Edocument.onselectstart=new%20Function%20(%22return%20false%22)%3C/script%3E%3Cinput%3E > > I think before this change, that wouldn't have prevented setting selection in a text field. > > Firefox doesn't appear to allow preventing selection start from this event at all. We should see what IE does. >
Thanks for an excellent reduction, Adele!
Dimitri Glazkov (Google)
Comment 3
2011-01-11 10:02:50 PST
IE behaves differently from pre-regression change. Here's the breakdown: 1) Pre-regression, WebKit behavior was to allow selection inside text input and textareas. 2) Post-regression, you can't even type into the field. 3) IE doesn't allow selection anywhere, though you can type into it all you want. Should I: a) restore original behavior, which seems a bit wrong (at least compatibility-wise) b) make WebKit behave like IE?
Dimitri Glazkov (Google)
Comment 4
2011-01-11 10:28:01 PST
Ew, even selectstart always returns true, keyboard selection still works in WebKit. I think this yak is too big to shave at the moment. I'll just unregress the behavior and leave the compatibility issue alone.
Dimitri Glazkov (Google)
Comment 5
2011-01-11 11:10:54 PST
Created
attachment 78556
[details]
Patch
Dimitri Glazkov (Google)
Comment 6
2011-01-11 11:13:43 PST
Created
attachment 78559
[details]
With better ChangeLog descriptions.
Eric Seidel (no email)
Comment 7
2011-01-11 11:39:55 PST
Comment on
attachment 78559
[details]
With better ChangeLog descriptions. View in context:
https://bugs.webkit.org/attachment.cgi?id=78559&action=review
OK.
> Source/WebCore/dom/Node.cpp:2628 > + getEventAncestors(ancestors, originalTarget.get(), event->isMutationEvent() || event->type() == eventNames().selectstartEvent ? StayInsideShadowDOM : RetargetEvent);
Should this check be its own function? Explaining why mutation events and selection start events are in thsi set?
Dimitri Glazkov (Google)
Comment 8
2011-01-11 11:50:45 PST
(In reply to
comment #7
)
> (From update of
attachment 78559
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=78559&action=review
> > OK. > > > Source/WebCore/dom/Node.cpp:2628 > > + getEventAncestors(ancestors, originalTarget.get(), event->isMutationEvent() || event->type() == eventNames().selectstartEvent ? StayInsideShadowDOM : RetargetEvent); > > Should this check be its own function? Explaining why mutation events and selection start events are in thsi set?
Great idea, will add a helper function with comments.
Dimitri Glazkov (Google)
Comment 9
2011-01-11 12:45:04 PST
Committed
r75536
: <
http://trac.webkit.org/changeset/75536
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug