Bug 37909 – Call sites of TextIterator constructor are difficult to read

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Shinichiro Hamaji]

As suggested in Bug 36419, TextIterator and CharacterIterator are instantiated with confusing boolean parameters. Let's use enum version of their constructor and eliminate the old constructors.

I'm planning to change the names of boolean members of these classes (e.g., m_emitCharactersBetweenAllVisiblePositions). They should be third person singular.

[Shinichiro Hamaji]

Created attachment 53918 [details]
Patch v1

[Shinichiro Hamaji]

Created attachment 53919 [details]
Patch v2

[Shinichiro Hamaji]

(In reply to comment #2)
> Created an attachment (id=53919) [details]
> Patch v2

I just modified the ChangeLog.

[Darin Adler]

Comment on attachment 53919 [details]
Patch v2

>  enum TextIteratorBehavior {
>      TextIteratorBehaviorDefault = 0,
> -    TextIteratorBehaviorEmitCharactersBetweenAllVisiblePositions = 1 << 0,
> -    TextIteratorBehaviorEnterTextControls = 1 << 1,
> +    TextIteratorBehaviorEmitsCharactersBetweenAllVisiblePositions = 1 << 0,
> +    TextIteratorBehaviorEntersTextControls = 1 << 1,
>      TextIteratorBehaviorEmitsTextsWithoutTranscoding = 1 << 2,
>  };

I think these repeat the word "Behavior" too much. I would call these:

    TextIteratorDefaultBehavior = 0,
    TextIteratorEmitsCharactersBetweenAllVisiblePositions = 1 << 0,
    TextIteratorEntersTextControls = 1 << 1,
    TextIteratorEmitsTextWithoutTranscoding = 1 << 2,

r=me, whether or not you change the names

[Eric Seidel (no email)]

Attachment 53919 [details] was posted by a committer and has review+, assigning to Shinichiro Hamaji for commit.

[Shinichiro Hamaji]

Landed in http://trac.webkit.org/changeset/58040

Thanks Darin for your review. I changed the name of enum values as you suggested.