1259672 - Clean up InternalFormEvent

Status: RESOLVED FIXED

(Core :: Widget, defect, P5)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

See bug 1259654 comment 0 for the detail.

Comment 1

[tyamaguchi.gentoo]

Attachment: 2-1-18.patch

patch for 1259672

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hi, Yamaguchi-san, what's the status of the patch? If you keep working, I'd like to wait your review request. Otherwise, I'd like take this bug.

Comment 3

[tyamaguchi.gentoo]

Sorry for late reply.
I'd like to request for review this patch.
Do I have something to do for the request?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yeah. You should do:

hg log -G -r central:
(check the revision of the change)
hg pull
hg rebase -s (the revision which you checked above) -d tip
hg up tip
./mach build

If the build fails, please modify the code and do:

hg commit -m "fix"
hg histedit (the revision of above)
(change the "pick" of the second line whose commit message is "fix" to "r" and save the fine and exit from the editor)

Then,

hg push -c (the revision) review

And press Enter for the confirmation.

Note that I'd like you to change the patch which you attached last week before the review rquest:

> diff --git a/dom/html/HTMLButtonElement.cpp b/dom/html/HTMLButtonElement.cpp
> --- a/dom/html/HTMLButtonElement.cpp
> +++ b/dom/html/HTMLButtonElement.cpp
> @@ -365,17 +365,17 @@ HTMLButtonElement::PostHandleEvent(Event
>        default:
>          break;
>      }
>      if (aVisitor.mItemFlags & NS_OUTER_ACTIVATE_EVENT) {
>        if (mForm && (mType == NS_FORM_BUTTON_SUBMIT ||
>                      mType == NS_FORM_BUTTON_RESET)) {
>          InternalFormEvent event(true,
>            (mType == NS_FORM_BUTTON_RESET) ? eFormReset : eFormSubmit);
> -        event.originator     = this;
> +        event.mOriginator     = this;

Please remove the redundant whitespaces before "=". One whisespace is enough.

> diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp
> --- a/dom/html/HTMLInputElement.cpp
> +++ b/dom/html/HTMLInputElement.cpp
> @@ -4146,17 +4146,17 @@ HTMLInputElement::PostHandleEvent(EventC
>          }
>          switch(mType) {
>          case NS_FORM_INPUT_RESET:
>          case NS_FORM_INPUT_SUBMIT:
>          case NS_FORM_INPUT_IMAGE:
>            if (mForm) {
>              InternalFormEvent event(true,
>                (mType == NS_FORM_INPUT_RESET) ? eFormReset : eFormSubmit);
> -            event.originator      = this;
> +            event.mOriginator      = this;

ditto.

> -  nsIContent *originator;
> +  nsIContent *mOriginator;

Currently, we align the |*| and |&| to the end of the type name. So, could you change this as:

nsIContent* mOriginator;

?

And also I'd like you to revisit bug 1259665 too. If you don't have much time, I'd take the patches, don't mind about that. And if you have some questions, feel free to ask me.

Thank you for your work!

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

FYI: If you've already set bookmark to the revision, you can specify it as the revision number. But be careful, |hg commit -m "fix"| takes the bookmark from the old commit to the new commit. So, you should use |hg log -G -r central:| to check the state of your commits.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

No response from tyamaguchi-san for long time. I'll take this.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1259672 Rename InternalFormEvent::originator to mOriginator

Review commit: https://reviewboard.mozilla.org/r/68866/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68866/

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8777274 [details]
Bug 1259672 Rename InternalFormEvent::originator to mOriginator

https://reviewboard.mozilla.org/r/68866/#review65938

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac8d7e05b65c
Rename InternalFormEvent::originator to mOriginator r=smaug

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ac8d7e05b65c