Summary: | Eliminate boolean arguments in HitTestRequest ctor and replace with enum | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Treat <manyoso> | ||||||||
Component: | Layout and Rendering | Assignee: | Adam Treat <manyoso> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | levin, staikos, zimmermann | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Adam Treat
2009-01-28 08:47:26 PST
Created attachment 27236 [details]
Eliminate bools from HitTestRequest
Patch to eliminate ugly boolean arguments to HitTestRequest ctor. Refactor and cleanup all the code that constructs and uses HitTestRequest.
enum RequestType { ReadOnly = 0x0, Active = 0x1, MouseMove = 0x2, MouseUp = 0x4, ClipToVisible = 0x8 }; I think it should be enum RequestType { ReadOnly = 0x1, Active = 0x2, MouseMove = 0x4, MouseUp = 0x8, ClipToVisible = 0x10 }; (right now readOnly() is never true.) (In reply to comment #2) > (right now readOnly() is never true.) Yes, and I also think ClipToVisible obfuscates the code a bit. How about calling it 'IgnoreClipping' or sth. along the lines, to not force most call-sites to add the HitTestRequest::ClipToVisible flag. It's also a dangerous default behaviour to force callers to enable it explicitely. There is only a Qt platform specific codepath, that actually needs ClipToVisible. Niko Created attachment 27238 [details]
New version to incorporate comments
ClipToVisible -> IgnoreClipping
Fix the bitflags.
Latest patch has teensy oops at EventHandler.cpp:733. I've fixed it. Do you need to update the EventHandler.h hitTestResultAtPoint to change clipToVisible and flip the bool? WebCore/page/EventHandler.cpp:727: result = mainFrame->eventHandler()->hitTestResultAtPoint(mainFramePoint, allowShadowContent, clipToVisible); Created attachment 27246 [details]
Fixing oops and flip bool
New version flips the bool and updates the ChangeLog
Comment on attachment 27246 [details]
Fixing oops and flip bool
Looks great, r=me, in case no layout test broke.
Adam, can you verify that it doesn't break any mac layout test? If not, I can try the patch first, just mail me.
Niko, unfortunately I can not verify as I don't own a mac. Please apply the patch and let me know. (In reply to comment #9) > Niko, unfortunately I can not verify as I don't own a mac. Please apply the > patch and let me know. We really need a trybot :-) Applying your patch as we speak, going to post results soon. Fixed and landed with r40486. |