Re: [Tails-dev] feature/unsafe_browser_name [Was: Iceweasel …

Nachricht löschen

Nachricht beantworten
Autor: intrigeri
Datum:  
To: The Tails public development discussion list
Alte Treads: Re: [Tails-dev] Iceweasel backports experiments
Betreff: Re: [Tails-dev] feature/unsafe_browser_name [Was: Iceweasel backports experiments]
Hi,

anonym wrote (21 Nov 2012 13:46:45 GMT) :
> I tested starting the Unsafe Browser in a current experimental build
> for every locale that can be selected in the menu in Tails Greeter,
> plus a few "Other" locales, and all of them displayed "Unsafe
> Browser" in the title as expected.


Awesome!

> Could someone that trusts my tests or want to replicate them (it
> took only a few minutes by running in a VM and saving/restoring
> state to Tails Greeter) please merge this into devel?


I do trust your tests, but nevertheless, I feel the need for
peer-review (surprise, surprise). So here's a quick static one.
I did not run the code.

The proposed change adds a dependency on the 7z command,
which is provided in Tails 0.14 by the p7zip-full package,
which we do not explicitly install.
I think this package should be added to
config/chroot_local-packageslists/tails-common.list, instead of
relying on file-roller pulling it as a dependency (this is the
case in Wheezy too, but IMHO, better safe than sorry and make the
dependency resolution explicit).

A refresh-translations run would be welcome, to update the PO files
for this script.

> I think I got the detection of which Iceweasel translation that will
> be used (depending on $LANG) right, which arguably is the most
> complex part of this procedure.


> +    SHORT=$(echo ${LANG} | cut -b1-2)
> +    LONG=$(echo ${LANG} | cut -b1-5 | tr _ -)


This seems to assume everything that can be in $LANG starts with the
xx_XX form, which is, unfortunately, not true.

I wonder how this script behaves when one chooses e.g. "Filipino" as
their language, and "Filipinas" as their region (=> fil_PH), or with
the Furlan language (iceweasel-l10n-fur-it).

A quick git grep shows this wrong assumption is already used in a few
other places in Tails, some of which written years ago, when we did
not support more than two languages => I just created
todo/fix_locales_form_wrong_assumption.

Given the already quite broken state of things, as long as the
proposed change does not break things in ugly ways for users of
non-xx_XX locales, I don't think it's a blocker, and I could find it
acceptable to add another instance of this bug.

> +    if [ -e "${EXT_DIR}/langpack-${SHORT}@???" ]; then
> +        PACK="${EXT_DIR}/langpack-${SHORT}@???"
> +        TOP=chrome
> +        REST=${SHORT}/locale
> +    elif [ -e "${EXT_DIR}/langpack-${LONG}@???" ]; then
> +        PACK="${EXT_DIR}/langpack-${LONG}@???"
> +        TOP=chrome
> +        REST=${LONG}/locale
> +    else
> +        PACK=${CHROOT}/usr/share/iceweasel/chrome/en-US.jar
> +        TOP=locale
> +        REST=
> +    fi


Are there languages for which both the short and long form exist?
If there are, then I'd like to be sure it works fine for them.
If there are not, then I'd rather see the code go from the most
specific (long form) to the least specific (short form), which would
be less surprising to my eye and probably more robust on the long
term. But given I don't know the problem domain, maybe I'm totally
missing the point :)

Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc