Bug 52608

Summary: Refactoring: EventHandler::handleTextInputEvent should accept an enum instead of three bools
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch rniwa: review+

Description Hajime Morrita 2011-01-17 21:14:06 PST
Coined at Bug 5241. Refactoring three bool parameters into one enum.
Comment 1 Hajime Morrita 2011-01-18 18:35:14 PST
Created attachment 79380 [details]
Patch
Comment 2 Ryosuke Niwa 2011-01-18 18:47:37 PST
Comment on attachment 79380 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79380&action=review

> Source/WebCore/page/EventHandler.cpp:2670
> -    RefPtr<TextEvent> event = TextEvent::create(m_frame->domWindow(), text, TextEvent::selectInputType(isLineBreak, isBackTab));
> +    RefPtr<TextEvent> event = TextEvent::create(m_frame->domWindow(), text, inputType);

Only concern I have with this change is that we may permit inputType to be TextEventInputPaste or TextEventInputDrop here. Can we add an assertion that this can only be Keyboard, LineBreak, or BackTab?
Comment 3 Ryosuke Niwa 2011-01-18 18:47:48 PST
Thanks for making this change!
Comment 4 Ryosuke Niwa 2011-01-18 18:51:59 PST
Comment on attachment 79380 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79380&action=review

> Source/WebCore/dom/TextEventInputType.h:9
> +/*
> + * Copyright (C) 2010 Google, Inc. All Rights Reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright

Oops, we shouldn't be claiming to be the author if we're just moving it here.
Comment 5 Ryosuke Niwa 2011-01-18 18:55:16 PST
Comment on attachment 79380 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79380&action=review

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:22369
>  				F3D4C47912E07663003DA150 /* InspectorBrowserDebuggerAgent.h in Headers */,
> +				A77B41A012E675A90054343D /* TextEventInputType.h in Headers */,

You probably want to put this in the right place as well.
Comment 6 Ryosuke Niwa 2011-01-18 18:59:53 PST
Comment on attachment 79380 [details]
Patch

(In reply to comment #4)
> Oops, we shouldn't be claiming to be the author if we're just moving it here.

I just realized you're the original author of this enum in http://trac.webkit.org/changeset/65287. I put my r+ back.

Please still correct the location of TextEventInputType.h in WebCore.xcodeproj/project.pbxproj as noted above.
Comment 7 Hajime Morrita 2011-01-18 19:52:35 PST
> I just realized you're the original author of this enum in http://trac.webkit.org/changeset/65287. I put my r+ back.
> 
> Please still correct the location of TextEventInputType.h in WebCore.xcodeproj/project.pbxproj as noted above.

Thanks! I'll land this after fixing project.pbxproj.
Comment 8 Hajime Morrita 2011-01-18 20:12:29 PST
Committed r76098: <http://trac.webkit.org/changeset/76098>