Closed Bug 1259672 Opened 4 years ago Closed 3 years ago

Clean up InternalFormEvent

Categories

(Core :: Widget, defect, P5)

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

(Whiteboard: [for Gecko Inside #7 in Mozilla Japan][tpi:+])

Attachments

(1 file, 1 obsolete file)

Attached patch 2-1-18.patch (obsolete) — Splinter Review
patch for 1259672
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.
Flags: needinfo?(tyamaguchi.gentoo)
Sorry for late reply.
I'd like to request for review this patch.
Do I have something to do for the request?
Flags: needinfo?(tyamaguchi.gentoo)
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!
Assignee: nobody → tyamaguchi.gentoo
Status: NEW → ASSIGNED
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.
Priority: -- → P5
Whiteboard: [good first bug][for Gecko Inside #7 in Mozilla Japan] → [good first bug][for Gecko Inside #7 in Mozilla Japan][tpi:+]
No response from tyamaguchi-san for long time. I'll take this.
Assignee: tyamaguchi.gentoo → masayuki
Mentor: masayuki
Whiteboard: [good first bug][for Gecko Inside #7 in Mozilla Japan][tpi:+] → [for Gecko Inside #7 in Mozilla Japan][tpi:+]
Attachment #8735085 - Attachment is obsolete: true
Comment on attachment 8777274 [details]
Bug 1259672 Rename InternalFormEvent::originator to mOriginator

https://reviewboard.mozilla.org/r/68866/#review65938
Attachment #8777274 - Flags: review?(bugs) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ac8d7e05b65c
Rename InternalFormEvent::originator to mOriginator r=smaug
https://hg.mozilla.org/mozilla-central/rev/ac8d7e05b65c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.