"Default hub encoding" isn't respected

[Victor Zamanian]

When I've set the "Default hub encoding" setting, for example to "CP1252 (Western Europe)" (in my case), this is not respected when joining a new hub with the Connect button or adding a hub to favorites by "/fav" in the message field.

I'm on Ubuntu 8.10, using version "1.0.2+cvs" of LinuxDC++ aaaand seems like I compiled it on 2008-11-10 (Nov. 10th).

Expected behavior:
Choosing a Default hub encoding should also be reflected in the encodings chosen when connecting to hubs with the Connect button and when adding favorite hubs with "/fav".

Actual results:
... seem to be that the system defaults are chosen -- UTF-8 in my case, which I assume is either the system default or a hard-coded default.

[Razzloss (razzloss)]

Haven't noticed this behavior before. Default encoding should be used when connecting via connect button. Favorite hubs use their own encoding settings (defined in fav. hub properties), which I guess by default is set to System Default (UTF-8 on most systems?).

In my opinion the System default in favorite hubs should be replaced by "Default encoding" which would follow the encoding set in the preferences. System default almost always is a wrong choice (as it often is UTF8 and that won't work in NMDC hubs)

--RZ

[qwertitis (qwertitis-deactivatedaccount)]

I agree. The semantics of System Default is confusing, since it is truly the system default, not whatever is set in the preferences. There should be a special element in the favorite hub setting for using the current preference. System default is kind of useless in favorite hubs, just like you said. It doesn't make sense that the hub would be correcting itself to whatever system encoding the connecting user has.

[Victor Zamanian (victorz)]

Razzloss:
From my experience, when you select the wrong encoding for a hub, only messages that do not contain the non-supported characters come through to one's own client and are shown in the main chat. I reported this bug in the middle of the night, so it might just have been a tidal low in messages on that hub, which normally has loads of chatterboxes in it. :-) Therefore I'm sorry about the, most probably erroneous, reporting of the bug when the default encoding isn't respected. In short, "Connect-button probably *does* respect default encoding. My bad."

But when I "/fav" a hub, I definitely feel that the default hub encoding I've chosen should be selected for the favorite hub entry that is produced with the "/fav" command. I hope I'm not the only one who feels that way. Maybe there's a reason why it is set to the UTF-8 system default? I dunno. :-) Just a though, because I always have to go into the settings for each hub I make a favorite of to select a new encoding. (... I also for some reason find myself making lots of hubs favorites and resetting my settings. :D)

Thanks for your time!

[qwertitis (qwertitis-deactivatedaccount)]

Makes sense. Seems like /fav won't save current hub's encoding into the fav entry, this surprises me.

[Steven Sheehy (steven-sheehy)]

Let me know if you can think of something better than "Global hub default" for the favorite hubs drop down.

[qwertitis (qwertitis-deactivatedaccount)]

I've let it slip earlier, but please don't change things and commit them under my name.

[Victor Zamanian (victorz)]

Steven Sheehy:
"Let me know if you can think of something better than "Global hub default" for the favorite hubs drop down."

Hmm. How about "Default hub encoding" (as it actually says in the Preferences)? It would probably be nicer if things that *are* the same thing are *called* the same thing. ;-)

[Steven Sheehy (steven-sheehy)]

>I've let it slip earlier, but please don't change things and commit them under my name.

I usually edit patches myself to fit them to coding standards, add comments, improve code readability, etc. It is my responsibility as maintainer to ensure the code is clear and consistent. It is generally quicker to change them myself than to ask the submitter to change them, but I can understand what you mean. In the future I will try to force patch writers to fix their own patches and if it does not meet my expectations I will reject it. On the other hand, I expect patch writers like yourself to follow the coding standards more closely.

> Hmm. How about "Default hub encoding" (as it actually says in the Preferences)? It would probably be nicer if things > that *are* the same thing are *called* the same thing. ;-)

I didn't choose "Default hub encoding" because it would be redundant to have encoding twice, once for the label and once inside the drop down box. "Encoding: Default hub encoding" sounds silly.

[Victor Zamanian (victorz)]

It may sound silly but it's consistent and maps directly to what it represents. I also imagine it's rare to speak the words: "Encoding: Default hub encoding". I picture a possible scenario going down as maybe: "Hmm what hub encoding should I use/Hey Bob! What hub encoding do you want/should I choose? Weeeell I'll go with the default hub encoding." Or something?

In short: Nah, should be fine for now until we can figure out a better term. It's worse the way it is now, where the settings say one thing and the favorites' preferences window introduces a completely new term -- "Global". Let me know what you think.

[Victor Zamanian (victorz)]

Also, why is there a new "Global hub default" option in the drop-down list? What I pictured when I filed the ticket was that when I /fav'ed a hub, it wouldn't put "Global hub default" in as Encoding, but the actual *value* of what the "Default hub encoding" was set to.

If one hasn't touched the Default hub encoding setting, one might not pay attention to the "Encoding: " option in the favorite hub settings either, and if one *has* changed the setting in the main preferences, one is likely to recognize one's previous choise of default encoding in the "Encoding: " option.

But maybe it's best this way? If one decides to change the default hub encoding and one wants all of one's favorite hubs to follow? HM. Maybe it could be called "Follow hub default" to ditch the extra "encoding"? :O And that option in itself would be made default whenever one would add a favorite, no matter the method, in the favorite's preferences (as I believe it is now).

[qwertitis (qwertitis-deactivatedaccount)]

@steven: Yes, my thoughts exactly

@zamanian: Well, you could have suggested this to me one week ago when I had written the patch instead of waiting until now, or didn't you realize I had implemented this? What you're calling for is to remove the special list item from fav hubs totally. Which kind of makes sense in one way -- but will strand us up with a lot of "i've changed the setting (in prefs) but nothing's changed" questions regarding connecting to hubs (fav'd obviously).

I called the fav-hub list item "User preference" (in my branch) because that is what it is. Global hub default sounds like some world-global defacto standard, and "default hub encoding" -- it isn't default (as in "factory setting") - the value depends on your preference.

I can also suggest that we remove the list item from fav hubs and add a checkbox instead: "Override user preference". Same semantics as current branch, different interface. This would be easier to i18nize too ;)

[Victor Zamanian (victorz)]

@individ:

removing the favorite hub encoding preference option isn't a good idea, no. I agree that the naming is difficult. But I think at least I would prefer the checkbox behavior, and after a hub has been "favorited" the checkbox would be unchecked by default (i.e. not overriding the default encoding). Maybe even graying out the list of char encoding alternatives until the user checks the override checkbox.

The obvious alternative is to keep it as it is and just change the wording of the fav-pref-encoding option from "Global hub default" to maybe "Follow default preference" or "Follow preferred encoding" or something. But like I said, my vote is with the checkbox thingaling.

[Steven Sheehy (steven-sheehy)]

I like the idea of an "Override default hub encoding", but that would only solve the naming in the dialog. We would still need to put some value inside the treeview to indicate to the user that it is using the default hub encoding.

[Victor Zamanian (victorz)]

@Steven: I don't know if I understand the problem you just described. From what I understand, the problem is that the user isn't properly informed that they are using the Default hub encoding. In that case it would maybe be possible to skip the previously described checkbox and instead use two radio buttons, one of which says "Use default hub encoding" and the other says "Use custom encoding: " (or something similar but better). Excuse me if I misunderstand the problem. :-)

[qwertitis (qwertitis-deactivatedaccount)]

@Victor: I think Steven means the content of the treeview column "Encoding" in the favorite hubs tab. (Not the fav hub dialog)

Using radio buttons would let us describe what happens if the encoding isn't overridden, which could be a good thing. But the setting isn't multi-choice, there are only to options - overridden or not.

In general I think the idea of which widget to use is like this: On/off: checkbox, 3 choices: radio button group, >3: combobox. (This is on Windos, in my experience many GNOME apps use comboboxes instead of radio groups, regardless of size.) Based on this I think a checkbox is preferred.

A problem with the proposed interface together with the current core backend is that we won't be able to store whatever encoding is chosen while the setting isn't overridden. So unchecking the override checkbox and reopening the dialog won't be able to bring the same encoding string back. Throwing away user input should be avoided since it makes users feel bad. I think a change is needed in the core to be able to save the "override" flag in fav hubs.

[qwertitis (qwertitis-deactivatedaccount)]

I'll try to make something of this today.

[Steven Sheehy (steven-sheehy)]

I don't think we should change it if it requires to change the core. While I can see the advantage, I don't mind it the way it is now. I just feel the name should be changed to something better.

[qwertitis (qwertitis-deactivatedaccount)]

Renamed Global hub default to Default.
Added override checkbutton.
Layout of fav hub dialog now looks like a GTK app.

I've done some other things with fav hubs but I figured it could be a separate patch. I'm opening a bug for that.

[Steven Sheehy (steven-sheehy)]

Patch review:
1) This should've been a new bug since you basically rewrote the fav hub dialog.
2) Not sure what was "un-GTK" about the old dialog, but the new one still needs some improvement. Please revert to the old style of having a GtkTable inside the GtkFrame. If you don't like the shadow around the GtkFrame you can turn it off, no need to replace the entire thing with one big table. With your approach it leaves a bunch of placeholders in the glade and using an entire row as a section label is not a pretty solution. You can still can still align the text box labels on the left using the old style.
3) First time you open fav hub dialog "Override default encoding" is not checked yet the encoding box is still editable. Checking and un-checking it becomes un-editable. Value inside box should be the "Default hub encoding".
4) When "Override default encoding" is not checked and the box is not editable, if you change the "Default hub encoding" in the preferences it does not get changed in the fab hubs dialog until you check "Override default encoding" again.
5) What is the purpose of putting N_() instead of _()? Can these strings not be translated until runtime?
6) Not an issue with your code since it was missing before, but can you please add params["Password"] to saveParams using the same logic to populate it from getFavHubParams_client? Without it the treeview doesn't get updated after editing.

[qwertitis (qwertitis-deactivatedaccount)]

Quick reply, cannot put any effort on this or any other issue in January, but I'll come back to it later.

1. Ok.
2. Using a single GtkTable, I can get all the widgets to line up even over the groups, which makes it look more harmonic. That is not my own idea though, I got it from the Gnome HIG (without mentioning how to accomplish it properly of course). I could repent, obviously this is a matter of taste.
3. Glaring error, that's something I ought to have seen. Added to todolist.
4. Nice catch. Added to todolist.
5. The string is written to file, and translating would break favs for users who upgrade to a translated version. That is my reasoning. I should really make it pass through gettext somewhere on the way to the user, though.
6. Ok. Added to todolist.

Thanks for the review, it is great to get critique and I'll do my best to make the changes necessary. We're celebrating a beautiful white Christmas this year, and I hope you are finding some time to relax too!

[Steven Sheehy (steven-sheehy)]

2) Actually, I didn't even notice that the two GtkTables weren't aligned since the difference was very slight. It should be fine to use a single GtkTable then. I would just like to see it slimmed down to two columns and use some other means to indent the text entry labels like xpad.
5) I'm not sure using N_() will work for that purpose. It would cause it to not break the first time after people upgrade, but it would also cause it to not work for subsequent times if you store the ENCODING_DEFAULT translated text in Favorites.xml. If you don't store the translated text it should be fine, but then people can't edit their Favorites.xml by hand. We're probably going to have other breakage due to internationalizing, so we shouldn't necessarily sacrifice keeping the code as simple as possible. Just give the users a warning to backup beforehand.

[Steven Sheehy (steven-sheehy)]

Hi David, any progress on an updated patch? I would like to get this committed soon. If you would like, I can make the changes myself if you're busy.

[qwertitis (qwertitis-deactivatedaccount)]

Steven Sheehy wrote:
> Hi David, any progress on an updated patch? I would like to get this
> committed soon. If you would like, I can make the changes myself if
> you're busy.
>
>
No, I won't have time for it. Go ahead.