Bug 43893 - [Refactoring] TextEvent class has to many flags
Summary: [Refactoring] TextEvent class has to many flags
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 43778
  Show dependency tree / graph
 
Reported: 2010-08-11 20:15 PDT by Hajime Morrita
Modified: 2010-08-12 18:45 PDT (History)
1 user (show)

See Also:

Attachments
Patch (8.09 KB, patch)
2010-08-11 20:21 PDT, Hajime Morrita 		no flags Details | Formatted Diff | Diff
Patch (8.44 KB, patch)
2010-08-12 04:29 PDT, Hajime Morrita 		tony: review+
Details | Formatted Diff | Diff
Show Obsolete (1) 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 2010-08-11 20:15:48 PDT 
From feedback on https://bugs.webkit.org/show_bug.cgi?id=43778
Comment 1 Hajime Morrita 2010-08-11 20:21:15 PDT 
Created attachment 64182 [details]
Patch
Comment 2 Ryosuke Niwa 2010-08-12 00:39:36 PDT 
Comment on attachment 64182 [details]
Patch

> --- a/WebCore/dom/TextEvent.cpp
> +++ b/WebCore/dom/TextEvent.cpp
...
>  TextEvent::TextEvent(PassRefPtr<AbstractView> view, const String& data, PassRefPtr<DocumentFragment> pastingFragment,
> -                     bool isPaste, bool shouldSmartReplace, bool shouldMatchStyle)
> +                     TextEvent::InputType inputType, bool shouldSmartReplace, bool shouldMatchStyle)
>      : UIEvent(eventNames().textInputEvent, true, true, view, 0)
> +    , m_inputType(inputType)
>      , m_data(data)
> -    , m_isLineBreak(false)
> -    , m_isBackTab(false)
>      , m_pastingFragment(pastingFragment)
> -    , m_isPaste(isPaste)
>      , m_shouldSmartReplace(shouldSmartReplace)
>      , m_shouldMatchStyle(shouldMatchStyle)

This constructor looks too general to me. It might be better to have two constructors one for pasting and another for other input types.
Comment 3 Hajime Morrita 2010-08-12 04:29:48 PDT 
Created attachment 64207 [details]
Patch
Comment 4 Hajime Morrita 2010-08-12 04:30:46 PDT 
Hi Ryosuke, thank you for your quick review!
I updated the patch.

> This constructor looks too general to me. It might be better to have two constructors one for pasting and another for other input types.
Agreed and fixed to split the constructor into 2.
Comment 5 Tony Chang 2010-08-12 09:58:07 PDT 
Comment on attachment 64207 [details]
Patch

Great! Thanks for doing this.
Comment 6 Hajime Morrita 2010-08-12 18:45:08 PDT 
Committed r65287: <http://trac.webkit.org/changeset/65287>