1239371 - Consider state of boolean values for marionette.logging preference

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

In my previous patch for bug 1239363 the marionette.logging preference is treated in a defined/undefined state, and it doesn’t look specifically at the state of the variable itself.

This means i.e. that if it is set to false, logging will still be enabled which was not the original behaviour.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: Bug 1239371 - Consider state of boolean values for marionette.logging pref; r?AutomatedTester

The patch for bug 1239363 treated the marionette.logging preference as
a defined/undefined state, and didn't look specifically at the state of
the variable itself.

This meant that if it was set to false, logging would still be enabled.
As this was not the original behaviour were adding backwards compatibility
for, this patch corrects this problem by making false equivalent to the
default (Log.Level.Info).

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

Comment 2

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8707545 [details]
MozReview Request: Bug 1239371 - Consider state of boolean values for marionette.logging pref; r?AutomatedTester

Please read the commit message which explains the changes.  Does this make sense to you, Maja?

We could make false be equivalent to something less fine-grained, such as Log.Level.Warn or Log.Level.Fatal, but I’d preferably want fewer defaults.

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8707545 [details]
MozReview Request: Bug 1239371 - Consider state of boolean values for marionette.logging pref; r?AutomatedTester

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

Comment 4

[David Burns :automatedtester]

Comment on attachment 8707545 [details]
MozReview Request: Bug 1239371 - Consider state of boolean values for marionette.logging pref; r?AutomatedTester

https://reviewboard.mozilla.org/r/30751/#review27667

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/54427ab62f79

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/54427ab62f79