Make NS_TEXTRANGE_* an enum class

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({inputmethod})

Trunk
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(7 attachments)

58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
For making our code clearer by the stronger type check, we should change the anonymous enum for NS_TEXTRANGE_* to enum class whose name is "TextRangeType" and whose type is "RawTextRangeType" which is an alias of uint8_t.

Additionally, this also adds some utility methods for them.

Note that some lines which are changed by this patch become over 80 characters but it will be fixed by the following patches.

Review commit: https://reviewboard.mozilla.org/r/57606/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57606/
Attachment #8759709 - Flags: review?(bugs)
Attachment #8759710 - Flags: review?(bugs)
Attachment #8759711 - Flags: review?(bugs)
Attachment #8759712 - Flags: review?(bugs)
Attachment #8759713 - Flags: review?(bugs)
Attachment #8759714 - Flags: review?(bugs)
Attachment #8759715 - Flags: review?(bugs)
Comment on attachment 8759709 [details]
Bug 1277756 part.1 Make anonymous enum for NS_TEXTRANGE_* to an enum class named "TextRangeType"

https://reviewboard.mozilla.org/r/57606/#review54418

::: widget/WidgetEventImpl.cpp:106
(Diff revision 1)
> +    case TextRangeType::NS_TEXTRANGE_CONVERTEDTEXT:
> +      return "NS_TEXTRANGE_CONVERTEDTEXT";
> +    case TextRangeType::NS_TEXTRANGE_SELECTEDCONVERTEDTEXT:
> +      return "NS_TEXTRANGE_SELECTEDCONVERTEDTEXT";
> +    default:
> +      return "Invlaid TextRangeType";

Invalid
Attachment #8759709 - Flags: review?(bugs) → review+
Attachment #8759710 - Flags: review?(bugs) → review+
Comment on attachment 8759710 [details]
Bug 1277756 part.2 Rename TextRangeType::NS_TEXTRANGE_UNDEFINED to TextRangeType::eUninitialized

https://reviewboard.mozilla.org/r/57608/#review54422
Comment on attachment 8759711 [details]
Bug 1277756 part.3 Rename TextRangeType::NS_TEXTRANGE_CARETPOSITION to TextRangeType::eCaret

https://reviewboard.mozilla.org/r/57610/#review54426

::: widget/TextRange.h:134
(Diff revision 1)
>   ******************************************************************************/
>  
>  enum class TextRangeType : RawTextRangeType
>  {
>    eUninitialized = 0x00,
> -  NS_TEXTRANGE_CARETPOSITION = 0x01,
> +  eCaret = 0x01,

I think I'd prefer eCaretPosition, but if there is some reason for shorter eCaret, fine. Up to you.
Attachment #8759711 - Flags: review?(bugs) → review+
Comment on attachment 8759712 [details]
Bug 1277756 part.4 Rename TextRangeType::NS_TEXTRANGE_RAWINPUT to TextRangeType::eRawClause

https://reviewboard.mozilla.org/r/57612/#review54428
Attachment #8759712 - Flags: review?(bugs) → review+
Comment on attachment 8759713 [details]
Bug 1277756 part.5 Rename TextRangeType::NS_TEXTRANGE_SELECTEDRAWTEXT to TextRangeType::eSelectedRawClause

https://reviewboard.mozilla.org/r/57614/#review54432
Attachment #8759713 - Flags: review?(bugs) → review+
Attachment #8759714 - Flags: review?(bugs) → review+
Comment on attachment 8759714 [details]
Bug 1277756 part.6 Rename TextRangeType::NS_TEXTRANGE_CONVERTEDTEXT to TextRangeType::eConvertedClause

https://reviewboard.mozilla.org/r/57616/#review54434
Comment on attachment 8759715 [details]
Bug 1277756 part.7 Rename TextRangeType::NS_TEXTRANGE_SELECTEDCONVERTEDTEXT to TextRangeType::eSelectedClause

https://reviewboard.mozilla.org/r/57618/#review54442
Attachment #8759715 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] (high review load, please consider other reviewers) from comment #15)
> Comment on attachment 8759711 [details]
> Bug 1277756 part.3 Rename TextRangeType::NS_TEXTRANGE_CARETPOSITION to
> TextRangeType::eCaret
> 
> https://reviewboard.mozilla.org/r/57610/#review54426
> 
> ::: widget/TextRange.h:134
> (Diff revision 1)
> >   ******************************************************************************/
> >  
> >  enum class TextRangeType : RawTextRangeType
> >  {
> >    eUninitialized = 0x00,
> > -  NS_TEXTRANGE_CARETPOSITION = 0x01,
> > +  eCaret = 0x01,
> 
> I think I'd prefer eCaretPosition, but if there is some reason for shorter
> eCaret, fine. Up to you.

Because I want to support "wide" caret (bug 979112). Then, the eCaret means not position, it becomes range.
Comment on attachment 8759709 [details]
Bug 1277756 part.1 Make anonymous enum for NS_TEXTRANGE_* to an enum class named "TextRangeType"

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57606/diff/1-2/
Comment on attachment 8759710 [details]
Bug 1277756 part.2 Rename TextRangeType::NS_TEXTRANGE_UNDEFINED to TextRangeType::eUninitialized

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57608/diff/1-2/
Comment on attachment 8759711 [details]
Bug 1277756 part.3 Rename TextRangeType::NS_TEXTRANGE_CARETPOSITION to TextRangeType::eCaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57610/diff/1-2/
Comment on attachment 8759712 [details]
Bug 1277756 part.4 Rename TextRangeType::NS_TEXTRANGE_RAWINPUT to TextRangeType::eRawClause

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57612/diff/1-2/
Comment on attachment 8759713 [details]
Bug 1277756 part.5 Rename TextRangeType::NS_TEXTRANGE_SELECTEDRAWTEXT to TextRangeType::eSelectedRawClause

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57614/diff/1-2/
Comment on attachment 8759714 [details]
Bug 1277756 part.6 Rename TextRangeType::NS_TEXTRANGE_CONVERTEDTEXT to TextRangeType::eConvertedClause

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57616/diff/1-2/
Comment on attachment 8759715 [details]
Bug 1277756 part.7 Rename TextRangeType::NS_TEXTRANGE_SELECTEDCONVERTEDTEXT to TextRangeType::eSelectedClause

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57618/diff/1-2/
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccdb440b7b7e
part.1 Make anonymous enum for NS_TEXTRANGE_* to an enum class named "TextRangeType" r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/93278105babf
part.2 Rename TextRangeType::NS_TEXTRANGE_UNDEFINED to TextRangeType::eUninitialized r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/41c18a6bdd84
part.3 Rename TextRangeType::NS_TEXTRANGE_CARETPOSITION to TextRangeType::eCaret r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/04cc8a274c7f
part.4 Rename TextRangeType::NS_TEXTRANGE_RAWINPUT to TextRangeType::eRawClause r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0a3c242becf
part.5 Rename TextRangeType::NS_TEXTRANGE_SELECTEDRAWTEXT to TextRangeType::eSelectedRawClause r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/0801a85a04be
part.6 Rename TextRangeType::NS_TEXTRANGE_CONVERTEDTEXT to TextRangeType::eConvertedClause r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1db300f075e
part.7 Rename TextRangeType::NS_TEXTRANGE_SELECTEDCONVERTEDTEXT to TextRangeType::eSelectedClause r=smaug
You need to log in before you can comment on or make changes to this bug.