1147576 - profile use is not implemented for fennec in mozregression

Status: RESOLVED FIXED

(Testing :: mozregression, defect)

Description

[Julien Pagès (:parkouss)]

Right now, if we pass a profile argument (or preferences), this won't be used if the application being tested is fennec.

We should fix this by sending the profile over the device's sdcard.

Will provided a link in eideticker where this is done: https://github.com/mozilla/eideticker/blob/master/src/eideticker/eideticker/runner.py#L138. This is provided as an example because it uses the old devicemanager api, and we have to use mozdevice here.

Comment 1

[Julien Pagès (:parkouss)]

Attachment: use profile (and profile prefs) for fennec

Hey, I did not tested it on a real device. Could you review it and test it Will ? (You said you had an idea for testing this...) Thanks!

Comment 2

[William Lachance (:wlach)]

Comment on attachment 8583405 [details] [review]
use profile (and profile prefs) for fennec

Hmm, I tested this and it seems to more or less work? I used the layers.low-precision-opacity setting from eideticker to see if the checkerboarding behaviour would be disabled. I can tell that the physical file user.js has the right value, but there was some strange behaviour I noticed while testing.

Namely, it only works if I specify "0.0" for the value (i.e. "--pref layers.low-precision-opacity:0.0"). Other values which seem like they should work, don't. e.g. "--pref layers.low-precision-opacity:0". This works for Eideticker, I don't know why it doesn't here.

I'd probably suggest merging this, as it's definitely better than before. However I wouldn't mind doing some more investigation of what's going on at some point. I suspect a bug in mozprofile more than anything else.

Comment 3

[Julien Pagès (:parkouss)]

Thanks for testing! And you found a good point to investigate.

I merged this as I also think it is not related to this patch directly, and I will investigate a bit more on the pref passing argument on firefox, to see if we also have the bug here.

Comment 4

[Julien Pagès (:parkouss)]

Hmm, it appears that in my about:config on firefox I do not have any preference that is shown as type float (only boolean, string and integers). The property you specified seems to be float, as I can see here: https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPrefs.h#298.

I found in the eideticker README.md, we have to pass:

--extra-prefs "{gfx.color_management.enablev4: true}"

This seems to be a real json object, that will be json decoded (https://github.com/mozilla/eideticker/blob/master/src/eideticker/eideticker/options.py#L112). BTW doc is wrong I think, one should write --extra-prefs "{'gfx.color_management.enablev4': true}" to be json valid (at least with standard json on python 2.7)

At the oppposite, mozprofile require:

--pref "gfx.color_management.enablev4: true"

And this is not a json object, and not decoded as such: http://mozbase.readthedocs.org/en/latest/_modules/mozprofile/prefs.html#Preferences.cast

and this cast function does not handle float at all by the way. Maybe this is our bug ?


The problem here is that with mozprofile when you entered "--pref layers.low-precision-opacity:0.0", the resulting pref value is a string, eg:

('gfx.color_management.enablev4', '0.0')

and in case you pass "--pref layers.low-precision-opacity:0", this is an int:

('gfx.color_management.enablev4', 0)

So I wonder how all this works. Semms that int is not acceptable for a float, but string representing a float is...


If this is the cause, maybe a possible fix for mozprofile - without breaking compatibility - is to try to transform first the --pref value with a json decoder, by adding "{" and "}" around the argument we got (and quotes around the dict key). If that fail, fall back to the already implemented cast thing...

Comment 5

[Julien Pagès (:parkouss)]

Ah, seems that mozprofile does not allow float at all: http://mozbase.readthedocs.org/en/latest/_modules/mozprofile/prefs.html#Preferences.read_json

raise PreferencesReadError("Only bool, string, and int values allowed")

Comment 6

[William Lachance (:wlach)]

The --pref option in mozregression is quite new, I think we'd be safe changing it to take a json dictionary of preferences. In fact I'd say it's the right thing to do. :)

I guess in the case of "0.0", it is actually most appropriate to pass it as a string, since it seems like Mozilla's about:config preference system does not support float. Still confused about why it works in Eideticker and not here.

Comment 7

[Julien Pagès (:parkouss)]

I wonder if the Preferences.cast class method in mozprofile should handle the float cases ? If the Preferences are also likely to be used with fennec profiles, I would say that it should.

:ahal, any thought on this ? (please read the comment 4) The issue we have here is that the Preferences.cast method does not handle all the possible values of a fennec profile (since type can be float in there), and I wonder if it should (maybe using a named argument, like allow_float to not break compatibility ?).

Comment 8

[Andrew Halberstadt [:ahal]]

Given that float isn't a valid type in about:config, I don't think we should support it in mozprofile either. What was wrong with keeping it as a string?

Comment 9

[Julien Pagès (:parkouss)]

Thanks Andrew for looking at this. :)

Well, the only problem is that the user will need to pass:

--pref "layers.low-precision-opacity:0.0"

E.g, this won't work:

--pref "layers.low-precision-opacity:0"

An other way to pass it would be to force string (but this is not obvious for floats, I would rather avoid that syntax here):

--pref "layers.low-precision-opacity:'0'"
--pref "layers.low-precision-opacity:'0.0'"


So this is not really user friendly, but we can deal with this. :)

@Will
Looking again at your proposition in comment 6, I'm not a big fan of changing the way a preference must be set. The main reason is that the mozprofile and mozprocess command line use the same format (--pref "PREF:VALUE"), so this is consistent. But as you mentionned, Eideticker uses another format (raw json) so I suppose we can do that too.

Another possibility is to just add some documentation, in the command line and/or the gh-pages to indicate how to deal with the float special case.

Will, your preference (eheh) ?

Comment 10

[William Lachance (:wlach)]

Sorry for being ridiculously late to reply to this. Based on my re-reading of the bug, I think some special-case documentation in gh-pages should be sufficient for the rare case that someone wants to specify a float here.

Comment 11

[Julien Pagès (:parkouss)]

Attachment: add some documentation for float prefs

add some documentation for float prefs.

Comment 12

[William Lachance (:wlach)]

Comment on attachment 8655480 [details] [review]
add some documentation for float prefs

I was thinking of website documentation but now that I think about it improving --help is a better way to go. Thanks!

Comment 13

[Julien Pagès (:parkouss)]

NP!

landed in https://github.com/mozilla/mozregression/commit/efa584fff699d96248e563582783dac974052523.