The WebCore::HitTestRequest class has too many boolean arguments in the constructor. This should be refactored to replace the boolean arguments with an enum.
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.