1336084 - misleading indentation introduced in bug 1120441

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Julien Cristau [:jcristau] (back April 22)]

The body of the if () block should be moved one level deeper.

Comment 1

[Julien Cristau [:jcristau] (back April 22)]

Attachment: Bug 1336084 - fix indentation after bug 1120441

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

Comment 2

[Andrzej Hunt :ahunt]

Comment on attachment 8832857 [details]
Bug 1336084 - fix indentation after bug 1120441

https://reviewboard.mozilla.org/r/109114/#review110330

Comment 3

[Andrzej Hunt :ahunt]

Comment on attachment 8832857 [details]
Bug 1336084 - fix indentation after bug 1120441

Clearing r+ as per:
https://bugzilla.mozilla.org/show_bug.cgi?id=1120441#c25

This fix is good, but I want to completely remove the conditional in question since I didn't want to land it in the first place (I'll provide a new patch to do that).

Comment 4

[Andrzej Hunt :ahunt]

Attachment: Bug 1336084 - Remove unneeded isForegrounded() call

I accidentally added this while working on a fix for Bug 1120441,
and forgot to remove it before landing. This call isn't harmful,
but it's unnecessary.

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

Comment 5

[Andrzej Hunt :ahunt]

Comment on attachment 8832857 [details]
Bug 1336084 - fix indentation after bug 1120441

https://reviewboard.mozilla.org/r/109114/#review110364

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8832984 [details]
Bug 1336084 - Remove unneeded isForegrounded() call

https://reviewboard.mozilla.org/r/109220/#review111510

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbfba62842ac
Remove unneeded isForegrounded() call r=sebastian

Comment 8

[Carsten Book [:Tomcat]]

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

Comment 9

[Andrzej Hunt :ahunt]

Comment on attachment 8832984 [details]
Bug 1336084 - Remove unneeded isForegrounded() call

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1120441.

[User impact if declined]: High probability of no user impact. However the code we are removing hasn't been tested extensively on nightly, and should therefore be removed to avoid unexpected issues.

[Is this code covered by automated tests?]: No.

[Has the fix been verified in Nightly?]: No.

[Needs manual test from QE? If yes, steps to reproduce]: No.

[List of other uplifts needed for the feature/fix]: None.

[Is the change risky?]: Low risk.

[Why is the change risky/not risky?]: This patch removes an unnecessary conditional. Nightly and beta both reflect the state after this patch (the unnecessary condition only landed on central, the patch that was uplifted to beta was fixed and doesn't contain the conditional, but aurora had the original patch uplifted directly from central).

[String changes made/needed]:None.

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8832984 [details]
Bug 1336084 - Remove unneeded isForegrounded() call

In order to align with beta, take this in aurora. Aurora53+.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/eabe078e6170