Bug 52608 - Refactoring: EventHandler::handleTextInputEvent should accept an enum instead of three bools
Summary: Refactoring: EventHandler::handleTextInputEvent should accept an enum instea...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-17 21:14 PST by Hajime Morrita
Modified: 2011-01-18 20:12 PST (History)
1 user (show)

See Also:

Attachments
Patch (18.97 KB, patch)
2011-01-18 18:35 PST, Hajime Morrita 		rniwa: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>