91721 - Language/Region Packs downloaded in 6.0x appear in View menu after upgrade but do not function

Status: VERIFIED FIXED

(Core :: Internationalization: Localization, defect, P1)

Description

[Jon Rubin]

Found on WinMe-Ja using 07-20-06-branch build.

Steps:

1) Download and install US 6.0x.
2) Go to View>Set Language/Region>Download More and download several language
and region packs, such as Italian (IT), Japanese (Japan), English (Japan), etc.
3) Download and install US 6.1 into same folder as 6.0x.
4) Go to View>Set Language/Region.  The language/region packs that were
downloaded and installed in 6.0x still appear.
5) Select one of the languages, such as Japanese (Japan).
6) Restart.  UI is still in English.

Note that at this time, a new ja-JP pack is available.  If that pack is
downloaded and installed, Japanese (Japan) will replace the 6.0x Japanese
(Japan) in the View menu and become functional (and JP Region will appear
separately).

Comment 1

[Jon Rubin]

If I install the 6.1 JA pack and then switch to JA UI, I can switch back to
English, but still not the other installed languages.

Comment 2

[Jon Rubin]

I double-checked with the PR1 build (06-07-13-branch); with that build, it was
still possible to change to languages original downloaded in 6.0x, such as French.

Tao, would the fix for bug 86807 prevent changing to languages that appear in
the View menu but were originally downloaded in 6.0x?

Comment 3

[Jon Rubin]

Copying some comments from bug 86807 that seem relevant:

 ------- Additional Comments From Jon Rubin 2001-07-10 22:55 -------

 Verified on branch using 07-10-05-branch on WinMe-Ja.  Verified according to
 Tao's suggested test plan by creating enUS, enJP, jaJP, and jaUS profiles on
 6.0x and 6.1, and jaJP profile on latest JA 6.1 branch build.  Opened each of
 these profiles in 07-10-05-branch and did full check of UI.  Some observations:

 1) Did not see ja UI in ja profiles; also did not see ja UI when switching to 
     
 ja language using View menu.
 2) JP profiles showed only JP Sidebar and Bookmarks; Search, Home button, and
 throbber remained set to US region.  This sounds similar to bug 90096, but the
 original problem in 90096 can no longer be reproduced by following the steps
 there (creating new US profile after downloading JP pack).
 3) Could not switch to JP region via View menu--see bug 89531.
 4) In Mail, Sort by Sender in View menu is blank.  Saw same problem in
 07-09-05-branch, which predates Tao's patch.  Filed bug 90266.

 The original problem reported in this bug--missing menu items--no longer occurs
 on the branch.




 ------- Additional Comments From Jon Rubin 2001-07-10 23:31 -------

 Filed bug 90279 for the UI problem in (1), and bug 90281 for the region problem
 in (2).

Comment 4

[Jon Rubin]

One more point on this:

There are two paths one can follow when upgrading from 6.0x:

1) Install to same folder used for 6.0x.  In this case, previously downloaded
lang/reg packs will appear in the Wiew menu, because the same chrome directory
is being used.  6.0x profiles can be used.

2) Install to a new folder.  In this case, previously downloaded lang/reg packs
will not appear in the View menu, because the original chrome directory is not
being used.  User will have to download 6.1 packs and only usable packs will
appear in the View menu.  6.0x profiles can be used.

I think a lot of users will not understand this distinction unless it is
addressed in Rel. notes.  Users would probably be more accepting of the idea of
old profiles reverting to the default UI language in #2 since old packs do not
appear in View.

Comment 5

[Jaime Rodriguez, Jr.]

This is core feature issue, right? Why is it assigned to rchen???

Nominate for TM 0.9.4

Comment 6

[Jon Rubin]

Jaime,

This was auto-assigned to rchen because I chose Localization as the Component. 
Who would be the best core person to assign this too?  Ben?

Comment 7

[Jaime Rodriguez, Jr.]

Msanz is currently talking to Tao, as to who should be the owner/driver for Packs.

Comment 8

[tao]

My patches for bug 86807 won't prevent outdated packs from appearing in the
"View" menu. Perhaps, we need to fix it from the chromeRegistry or the XUL code.
I'll take a look. This is not a localization issue, btw.-> tao

Comment 9

[Jon Rubin]

Added this to the RTM Branch Release Notes Tracking bug (bug 90577).

Comment 10

[tao]

I think we can uninstall out-dated packs when they are selected; I will give it
a shot.

Comment 11

[Jaime Rodriguez, Jr.]

Cindy - If this is valid. Its something we should fix for eMojo.

Comment 12

[Andreas Becker]

Adding nsBranch keyword.

Comment 13

[Andreas Becker]

mass change, switching qa contact from jonrubin to ruixu.

Comment 14

[tao]

Attachment: moz patch: display packs only when localeVersion match

Comment 15

[tao]

Attachment: ns patch: add locale version entities to dtd

Comment 16

[tao]

Hi, Hyatt and Waterson:


To solve the outdated provider packs problem in an upgraded client installation,
I add one more condition in the XUL template rule to check if the localeVersion
matches. Only packs with matching version number would display in the 
language/region selector. 

Would you please r & sr this patch if you agree with this approach? Also, I'd
like to suggest we do the same checking with the theme packs to avoid
user confusion.

Comments?

Comment 17

[David Hyatt]

sr=hyatt

I'm not sure we can do the same thing for skins.  I seem to recall that
marketing wanted to deliberately show you the outdated skins and then give you a
dialog pointing you to theme park to update.

tpringle, thoughts?

Comment 18

[tao]

Interesting point. Unfortunately, there is no UI to un-install language/content 
packs now. Maybe we can just grey out the outdated packs so people can't select
them. Let me see if we can do this via XUL template rule.

Comment 19

[Todd Pringle]

hyatt, yeah that was one thought we had, but i thought it was partially driven
by the fact that we didn't think we could actually remove them from view.  if we
can remove them from view, that is a different situation.  I'm not sure we
should do that with themes unless there is some convenient way to inform the
user what is happening (i.e. their themes are being removed because they are
incompatible with their new version) somewhere during the upgrade process.  

I actually tend to think that our current solution is a reasonable one - show
the old themes in the UI but give users a dialog telling them that they are
incompatible with the current version if they try to select one, and suggesting
that they uninstall.  If we could gray them out and still provide this
experience, and allow them to uninstall directly from the dialog (not sure if we
do that already?), that would be even better.  In any event, the current method
enables users be informed of what is going on.  Given the meager volume of
themes out there, this is sufficient for now - i.e. it doesn't make sense to
prompt people to look for updates on the theme park at this point.

Comment 20

[Jaime Rodriguez, Jr.]

Is there a way we can leverage the Themes UI for uninstall to addres this concern?

Comment 21

[Jaime Rodriguez, Jr.]

Sounds like Todd answered my question. My vote would be for consistency in
expereince here.

Comment 22

[Chris Waterson]

This patch looks okay to me: I'm no UE expert but having out-of-date content
packs show up as ``greyed out'' with no other explanation might be confusing. If
you do go that route, it might make sense to show the string ``(needs upgrade)''
next to the out-of-date content packs. You could do that with two template
rules, one which matches the current version, and a second ``default'' rule that
assumes any others are out-of-date; e.g.,

  <rule chrome:localeType="region" chrome:localeVersion="&content.version;">
    <!-- this rule matches up-to-date content packs -->
    <!-- treechildren, etc. -->
  </rule>
  <rule chrome:localeType="region">
    <!-- this rule ``falls through'' to match obsolete content packs -->
    <treechildren flex="1">
    <treeitem id="treechildren" uri="..." translation="true"
              nselected="false" >
    <treerow>
    <treecell class="outofdate"
     label="rdf:http://www.mozilla.org/rdf/chrome#displayName (needs upgrade)"
     value="rdf:http://www.mozilla.org/rdf/chrome#name"/>
    </treecell></treerow></treeitem></treechildren>
   </rule>

Of course, you'd want to be able to localize the string ``(needs upgrade)'' with
an entity. This, plus a style rule for |treecell.outofdate| that sets the text
color to whatever ``disabled'' is should do the trick...

Either way, r=waterson.

Comment 23

[tao]

The current content pack UI in Pref dlg is rather different from the theme one.
User's selection in the content pack does not take effect until users click on
the OK button of the Pref dlg.

To prompt the users when packs are outdated, we'll need to respond to user's
selection even they are committed. Same thing should happen to the language
dropdown list in Prefs dlg, profilemanager, and View menu. To sum up, we need to

  1. In Prefs dlg & ProfileManager: 

     a. respond to user selection right away in content pack and language 
        pack selection 
     b. add remove pack button to content pack tab
     c. Note that there no obvious way to remove the language pack since the
        anguage selector is a dropdown list (same thing to the one in
        Profile Manager)
  2. In the View menu, prompt users to remove packs via Prefs dlg. But, again,
     there is no proper UI to remove language in the Pref dlg.

CC german.

Comment 24

[tao]

Hi, chris:

Thanks for the tip and review. I'll play with it while awaiting input from german.

Comment 25

[german]

From the little I understand about the issue technically I would suggest that
while the implementation underneath is different the general notion (like in
Themes) to offer the user to visually remove the package from the menu (aka
'uninstall' from a user's perspective) is a reasonable appraoch. Agree with
Chris, just greying out the item without any explanation text seems not
sufficient to let user know what is going on.

Comment 26

[tao]

I'll grey out outdated pack and append " (needs upgrade)" to them. then!

Comment 27

[Jaime Rodriguez, Jr.]

Tao - Is this one ready?

Comment 28

[Jaime Rodriguez, Jr.]

Removing ME. PM --> Todd

Comment 29

[tao]

Unless we want to silently hide outdated packs, the patch is not ready, yet.
Remaining UE issues:

  1. there is no way to uninstall langpacks
  2. Is it sufficient to simply appending " (need upgrade)" to outdated packs 
     in the UI?

Comment 30

[Jaime Rodriguez, Jr.]

If there is a workaround, so that users can not use outdated packs, the PDT is
ok to wait on this fix. 

nsbranch- and 0.9.4.

Comment 31

[tao]

How about we prompt the user, when outdated packs are selected and should be 
uninstalled: "OK"-> uninstall it; "Cancel" -> don't switch it?

Let me see if this is feasible.

Comment 32

[tao]

reassign to new owner ->dbragg

Comment 33

[dbragg]

Ok, I've been playing with this a bit and I have a question.  Would it be wrong
to simply not show obsolete language packs to the user?  They would then go and
get the ones they want without having to add a bunch more UI and they wouldn't
be confused by selecting an obsolete lang pack and having it not work. Is it not
true that the only ones affected by this bug are those that download one
language (say en-US) and want to get packs for other, additional languages? 
Wouldn't that kind of user know that if they're getting an updated english
version, they'd have to get the updated packs?  If someone downloaded and
installed the "full" Japanese product wouldn't they get the latest Japanese
language pack?

If my assumptions are not correct please tell me.

Comment 34

[tao]

Don - See my first patch, "moz patch: display packs only when localeVersion match",
<http://bugzilla.mozilla.org/attachment.cgi?id=47606&action=view> does why you
suggest. We didn't go for it at the end. Please see comment #17, #19, #25. 

Comment 35

[dbragg]

Sorry.  Got so focused on the "fix" that I didn't re-read the comments.  Todd's
comment (#19) raises another question though.  Are we requiring that the pack be
uninstalled before an updated one is installed?  If not, do we still want to
provide UI for lang pack removal as German suggested?  Is that even within the
scope of this bug?  I'm working on setting up a meeting with the current UI
folks on this since I don't see an actual consensus on what should be done, plus
there's the issues of the way this is all presented in the different parts of
the program (Profile Manager, Edit/Preferences, View menu).

Comment 36

[tao]

Yes, let's design a solution to attack all issues instead of one at a time. We
should provide a means to remove outdated packs. It is a matter of where and when.

Comment 37

[Rui Xu]

Changed QA Contact to [e-mail]

Comment 38

[dbragg]

Posting patches.  I've done 2 separate diffs to make it easier to follow.  They
are broken down by their location in the mozilla tree.  They are:

mozilla/extensions/content-packs
mozilla/xpfe

The patch does several things.  
* It removes the language selection drop-down from the root of the
Preferences/Appearance menu.  
* It puts the language selection into the (new) Languages/Content section of the
Preferences/Appearnce menu in a listbox format instead of the dropdown.
* It changes the View menu so that instead of a submenu for Languages and
Content, it brings up the same dialog as Preferences/Appearance/Language and
Content.
* Finally, it lists obsolete language and content packs as "(needs update)" and
prevents the user from selecting them.

Comment 39

[dbragg]

Attachment: mozilla/extensions diff

Comment 40

[dbragg]

Attachment: mozilla/xpfe diff

Comment 41

[dbragg]

The original ns part of the patch is still valid.

Comment 42

[tao]

Comment on attachment 47607 [details] [diff] [review]
ns patch: add locale version entities to dtd

the version # needs update

Comment 43

[tao]

Comment on attachment 69142 [details] [diff] [review]
mozilla/xpfe diff

please don't forget to update the version in .dtd.
Better write a script to do
it..

Comment 44

[tao]

BTW, Don - would you please log bugs to track the issues I brought up? For example,

  1. highlight the current selection
  2. expand the appearance tree in the left pane.
  3. "uninstall" button
  4. sync up ProfileCreation screen.

Comment 45

[Alec Flett]

Comment on attachment 69141 [details] [diff] [review]
mozilla/extensions diff

sr=alecf
thanks for the comments too!

Comment 46

[Alec Flett]

Comment on attachment 69142 [details] [diff] [review]
mozilla/xpfe diff

everything looks good except you don't need the 
<menuitem...>
</menuitem>

(i.e. no nodes in between the open/close of the tag)
instead you should use the 
<menuitem ... /> syntax
sr=alecf

Comment 47

[dbragg]

Alec,
Actually, I had to do the </menuitem> separately for some reason.  When I just
closed it with a /> I got all these weird assertions and then the File and Edit
menus didn't show up at all.  As soon as I put in the separate </menuitem> those
problems went away.

Weird huh?

Comment 48

[Alec Flett]

that is wierd, but it seems very strange as well. Have we hit some odd edge case
in the XML parser, or was there just a typo? can you:
1) put a comment in XML directing people to this bug
2) copy & paste the assertions into this bug. 

At least if someone wants to fix it, that will make it easier.

Comment 49

[dbragg]

Fix checked in.  After this is verified, I'll go back to the syntax that Alec
suggested for the menuitem and post the Asserts.

Comment 50

[Jimmy Uen]

*** Bug 90092 has been marked as a duplicate of this bug. ***

Comment 51

[Jimmy Uen]

Verified with Mozilla 2002022703

Comment 52

[Dawn Endico]

removed the item for this bug from the release notes for 0.9.9 and beyond,
because the bug is fixed now. Let me know if you think the item should be 
re-added.

Comment 53

[Blake Ross]

When you make UI changes like this, Jatin needs to be notified so that help can
be updated.  (Please excuse me if you'd already done so elsewhere). 
Specifically, the "Setting Language Preferences" section now points to
nonexistent language selection UI in the Appearance panel.

Comment 54

[Jatin Billimoria]

Thanks for the heads up, Blake. The online help for MachV will address these
changes in UI for Languages, but it always helps to be informed about them so
that they don't sneak up on me.

Comment 55

[dbragg]

Yes, a very good catch Blake. Thank you.  I'm wondering, is there somewhere in
the mozilla documentation that talks about who to notify when certain types of
changes are made?  I didn't know about Jatin nor did any of my reviewers.  I
searched around and didn't find anything on mozilla.org.  If it's there and I
missed it please enlighten me.  If it's not there, I'd suggest adding it to the 
http://www.mozilla.org/docs/ page in the Misc. secion under Guidelines and
References.  Perhaps something like "Changing UI elements" or "Who to notify
when making changes".