1348415 - Use present tense for GeckoView interface method names

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

`onTitleChanged` should be `onTitleChange` in `ContentListener` and `onSecurityChanged` should be `onSecurityChange` in `ProgressListener`, to match other method names like `onPageStart` that use a verb's present tense.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Also, `public` is redundant for a method in an interface, so we should remove the 'public' keyword from method definitions in GeckoView interfaces.

Comment 2

[Nitish]

Attachment: Bug 1348415 - Use present tense for method names and remove redundant public modifiers for GeckoView interface method names

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

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8848717 [details]
Bug 1348415 - Use present tense for method names and remove redundant public modifiers for GeckoView interface method names

https://reviewboard.mozilla.org/r/121622/#review124046

Thanks for the patch! There are just a couple other places that need to be updated.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoView.java:172
(Diff revision 1)
>                  Log.d(LOGTAG, "handleMessage: event = " + event);
>              }
>  
>              if ("GeckoView:DOMTitleChanged".equals(event)) {
>                  if (mContentListener != null) {
>                      mContentListener.onTitleChanged(GeckoView.this, message.getString("title"));

Please update this line.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoView.java:193
(Diff revision 1)
>                  if (mProgressListener != null) {
>                      mProgressListener.onPageStop(GeckoView.this, message.getBoolean("success"));
>                  }
>              } else if ("GeckoView:SecurityChanged".equals(event)) {
>                  if (mProgressListener != null) {
>                      mProgressListener.onSecurityChanged(GeckoView.this, message.getInt("status"));

Please update this line.

Comment 4

[Nitish]

Comment on attachment 8848717 [details]
Bug 1348415 - Use present tense for method names and remove redundant public modifiers for GeckoView interface method names

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/121622/diff/1-2/

Comment 5

[Nitish]

Are changes required in the event messages "GeckoView:DOMTitleChanged" and "GeckoView:SecurityChanged". If so, then where are these messages passed as parameter 'event' to GeckoView.java:handleMessage(final String event, final GeckoBundle message,final EventCallback callback)?

Comment 6

[Nitish]

Are changes required in the event messages "GeckoView:DOMTitleChanged" and "GeckoView:SecurityChanged". If so, then where are these messages passed as parameter 'event' to GeckoView.java:handleMessage(final String event, final GeckoBundle message,final EventCallback callback)?
Thanks.

Comment 7

[Nitish]

Sorry. The comment came twice due to a poor internet connection.

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8848717 [details]
Bug 1348415 - Use present tense for method names and remove redundant public modifiers for GeckoView interface method names

https://reviewboard.mozilla.org/r/121622/#review124514

Thanks for your patch!

(In reply to Nitish from comment #5)
> Are changes required in the event messages "GeckoView:DOMTitleChanged" and
> "GeckoView:SecurityChanged". If so, then where are these messages passed as
> parameter 'event' to GeckoView.java:handleMessage(final String event, final
> GeckoBundle message,final EventCallback callback)?

You don't have to change that.

Comment 9

[Nitish]

Hi Jim, I was away for a while. Is there any other change to be made before the try server run?

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

No, your patch is good.

Comment 11

[Nitish]

Could you land the patch?

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ec9038f1b16
Use present tense for method names and remove redundant public modifiers for GeckoView interface method names r=jchen

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1ec9038f1b16