Summary: | REGRESSION(r71934) Can't type in search edit field on skin-one.com | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adele Peterson <adele> | ||||||
Component: | Forms | Assignee: | Dimitri Glazkov (Google) <dglazkov> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | dglazkov, eric, rniwa | ||||||
Priority: | P1 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 46015 | ||||||||
Attachments: |
|
Description
Adele Peterson
2011-01-10 21:51:19 PST
My booger. Will fix. (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! 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? 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. Created attachment 78556 [details]
Patch
Created attachment 78559 [details]
With better ChangeLog descriptions.
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? (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. Committed r75536: <http://trac.webkit.org/changeset/75536> |