GNOME Bugzilla – Bug 786818
EphyPasswordManager has hostname variables that contain security origins
Last modified: 2017-08-27 17:41:34 UTC
It's very important to use accurate terminology in our code, to avoid future programming mistakes. Previously we had a bunch of problems with "uri" variables in the form auth code that actually wanted to receive hostnames. This caused bugs when I modified the code and passed actual URIs instead of hostnames to parameters named uri, because what else was I going to do? :) So it's bad that EphyPasswordManager stores security origins in hostname variables. We should consistently rename these variables to origin.
Created attachment 358493 [details] [review] password-manager: Handle security origins only (no more URIs) Change the API of EphyPasswordManager to handle security origins only, thus clearing possible confusions about whether to call functions with URIs or origins. Also fix bad variable naming: "hostname" -> "origin".
Review of attachment 358493 [details] [review]: Well that was more work than I expected. Thanks. ::: lib/sync/ephy-password-record.c @@ +196,1 @@ g_param_spec_string ("hostname", We should change the property name too, right? @@ +205,2 @@ obj_properties[PROP_TARGET_ORIGIN] = g_param_spec_string ("formSubmitURL", And here too. @@ +274,1 @@ "formSubmitURL", target_origin, Ditto.
Michael Catanzaro: We can't change the property names, they have to same as in Firefox for sync to work.
(In reply to Alexander Mikhaylenko from comment #3) > Michael Catanzaro: We can't change the property names, they have to same as > in Firefox for sync to work. Exactly. We need to match the format exposed here: https://mozilla-services.readthedocs.io/en/latest/sync/objectformats.html#passwords GObject serialization works with properties and their values, so changing the property names will break the format expected on the sync storage server, thus making Firefox most probably not recognize the objects and ignore them when syncing.
Ah that's pretty cool. I forgot about that. OK then.
Attachment 358493 [details] pushed as 3960972 - password-manager: Handle security origins only (no more URIs)