Opened 4 years ago

Closed 2 years ago

#16412 closed bug (fixed)

Problems changing Decorators/ControlLooks

Reported by: bitigchi Owned by: jscipione
Priority: normal Milestone: R1/beta4
Component: Preferences/Appearance Version: R1/beta2
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description

Problem 1 & 2

Steps to reproduce:

  • Install flatstyle CL from HD
  • Change Decorator to Flat

System look changes to Flat immediately.

  • Restart
  • Go to Appearance preferences

Both Decorator and ControlLook report "Default" instead of Flat

In order to change the system look, it's sufficient to change the Decorator, what does the ControlLook do in this context? It's not doing anything!

Problem 3

Since Haiku original look is no longer in the list, it's not possible to revert to it from the menu. One has to click "Defaults" and it resets other settings as well!

Summary:

  1. Only Decorator setting works
  2. Changed theme is reported as Default instead of its own name
  3. It's not possible to revert to the Haiku original look

hrev54418

Attachments (1)

bug.mkv (5.0 MB ) - added by cafeina 3 years ago.
Video capture of the bug with a Spanish language locale system.

Change History (28)

comment:1 by Starcrasher, 4 years ago

I didn't manage to reproduce it fully but indeed Decorator is misidentified after reboot. https://github.com/unarix/haiku_darkstyle/issues/3
*Edit* After testing Be Decorator has same issue, so it seems a problem on Haiku side.
To get back to Default decorator, you need to select Flat (or Be) again in Appearence app then to close and to reopen the app and select Default.
I checked if Control look has same issue but it was correctly identified after reboot.
Decorator only changed borders for me, not full system look. Both seem good from this point. Did you think about removing old versions of these decorator and control look that you might have in /boot/home/config/non-pagkaged?
Note that wouldn't work at all using ThemeManager, paths to decorator are wrong in Flat themes files and TM doesn't handle Control look at all. https://github.com/unarix/haiku_darkstyle/issues/2

Tested with hrev5429

Last edited 4 years ago by Starcrasher (previous) (diff)

comment:2 by Starcrasher, 4 years ago

Maybe not related but Decorator name used to be translated (Default -> Par défaut) and it is no longer the case.

comment:3 by bitigchi, 4 years ago

IMO, the whole thing is so confusing. What's a ControlLook? What's a Decorator? What is a "Default"? It's not even possible to translate "ControlLook" properly.

It should just be one drop-down menu with title "System Theme", then two entries provided by default, "Haiku" and "Be".

comment:4 by Starcrasher, 4 years ago

Think of a window like a present picked on a list and containing a vase. Customer manages what is inside the package, the shape of the vase. Saler handles the aspect of the package, the kind of paper, the kind of ribbon. None manage colours, or functionnality. List said it should be a vase and the girl it is for wants everything to be a nuance of pink.

Decorators allow to change the look of the outside of the window. i.e. The look of the external borders and the tab. It is something inherited from Be, even if the setting was hidden at this time. This setting doesn't need a restart of apps and effect is immediate. It affects every apps no matter if they are native or not.

Haiku goes further and ControlLook allows to modify the look of what is inside the window. What is mainly modified is the aspect of controls (buttons, drop-down menus, sliders, internal tabs). To see effects of this setting apps need to be restarted. It only affects native apps or apps following Haiku style. I.e if you're not using Haiku widget style in Qt apps, they won't be impacted. In French, it has been translated as "Contrôleur d'aspect" which means aspect controller.

I think case of Decorator and ControlLook, it could be better to use "Haiku" or Haiku (Default) than "Default" indeed.

Even if, later, you were grouping these settings "System theme" would be a bad choice. A theme is a way more global thing. Look at what is managed by ThemeManager and what it plans to manage later. It includes colours, fonts, sounds, wallpappers and a lot more like icons or even skins for apps.

Personally I would rather group fonts and antialiasing tabs and allow to save independently coulour schemes, aspect and fonts. So people could mess with those settings without fear to lose what they already configured. Especially fonts settings that are important for different languages.

Another solution would be to include ThemeManager in Haiku and have a theme named Haiku. Of course for that, it should handle ControlLook, the few colours that aren't managed yet and maybe drop some plugins.

comment:5 by jscipione, 4 years ago

I thought I had fixed this in https://git.haiku-os.org/haiku/commit/?id=2e99f0e1fe64 try looking for my mistake there if I don’t get to this soon enough.

comment:6 by Starcrasher, 4 years ago

The behaviour changed a bit since that time. In fact, only "Default" value of Decorator is causing problems.
The settings file isn't updated with the new value and is not deleted either.

in reply to:  3 comment:7 by jscipione, 4 years ago

Replying to bitigchi:

IMO, the whole thing is so confusing. What's a ControlLook? What's a Decorator? What is a "Default"? It's not even possible to translate "ControlLook" properly.

It should just be one drop-down menu with title "System Theme", then two entries provided by default, "Haiku" and "Be".

The Decorator name comes from Design Patterns: Elements of Reusable Object-Oriented Software [0] page 196

Control Look should be called Façade from [0] page 208 but the person who named it didn't know the reference and now it is too late to change. We could change the menu field label in Appearance though.

Someday I'd like to add a color scheme list to Appearance which would allow you to create, save, and import color scheme sets making ThemeManager obsolete.

Master Theme switch is then possible.

Default should be renamed Haiku and DefaultDecorator should be renamed HaikuDecorator while DefaultDecorator class should be empty and point to HaikuDecorator. DefaultWindowBehaviour should also be renamed HaikuWindowBehavior (US spelling please) and then DefaultWindowBehavior class should be an unmodified child of HaikuWindowBehavior.

[0] http://www.uml.org.cn/c++/pdf/DesignPatterns.pdf

comment:8 by jscipione, 4 years ago

Owner: changed from nobody to jscipione
Status: newin-progress

comment:9 by jscipione, 4 years ago

Blocked By: 12994 removed

comment:10 by jscipione, 4 years ago

Resolution: fixed
Status: in-progressclosed

Fixed in hrev54512

comment:11 by Starcrasher, 4 years ago

Problems mentionned in summary 3 are still there in nightly hrev54513.
ControlLook setting works correctly whatever the value. Decorator setting isn't saved for 'Default' value, the others values are working fine.
You can set 'Be' or 'Flat' and see visual result immediately. You can change from 'Flat' to 'Be' or from 'Be' to 'Flat' without problem but, you can't go back to 'Default' without extreme mesures.
Actually to get back to 'Default', you have to set 'Default', to close "Appearance", to delete decorator_ settings file and reboot. Resetting all Appearance values or loading a theme that use default decorator with TM would also work.

As said in comment 6, what happen when you chose decorator 'Default' value is that the file /boot/home/config/settings/system/app_server/decorator_settings isn't updated with the new path nor is it deleted; so there's no effect visually.
If you close and reopen Appearence the setting is back to its previous value (either Flat or Be) which is consistent with the visual aspect and what's in the setting file. So loading is ok.

In fact, it is quite logical, since path to Haiku decorator is the default one, the value is empty, there's nothing to overide and so nothing to save. The problem is that you have to overide the previous value anyway. If you load a theme using Haiku decorator with TM, you can see that the string used to be set to 'Default' in that case.
message ./decorator_settings BMessage(0x0) {

decorator = string("Default", 8 bytes)

}

comment:12 by jscipione, 4 years ago

Resolution: fixed
Status: closedreopened

comment:13 by jscipione, 4 years ago

The path of the Default Decorator is “Default” and it should write that to your config file in /boot/home/config/settings/system/app_server/decorator_settings when you select Default from the pop up menu. Otherwise it writes the path to the selected decorator in the config file.

comment:14 by jscipione, 4 years ago

I am unable to reproduce this issue after hrev54512. I open Appearance, switch both my decorator and control look to Be. Reboot, when it comes back up the pop up menus each have Be selected. I change my decorator and control look back to Default, reboot, and the Appearance pop up menus have Default selected.

comment:15 by Starcrasher, 4 years ago

Checked again and decorator still not working with hrev 54538 x64. Reading your comment, I was a bit puzzled and I tried to understand why that difference of behaviour exists.
So, I changed momentarily my locale settings to English and then it worked. Switched back to French and it doesn't. I hope that helps.

comment:16 by bitigchi, 4 years ago

Probably it's the term "Default" messing things up. Do we really need to use it? Why not just Haiku and Be? The term "Haiku" is understandably the default.

comment:17 by Starcrasher, 4 years ago

It's not the term in itself otherwise it wouldn't work in English.
I guess that the code uses the translated name to know what's in use instead of using Default. Of course, it doesn't have an answer so nothing is done.
Obviously "Be" will never be a problem and for the moment, "Flat" isn't translated in French. But I'm not sure that it would work if it was translated by "Plat". So, in a way or another the code needs to be fixed.
Even if it is probably untranslated in all languages, I think that using "Haiku" wouldn't work. It's because default settings aren't treated the same way.

comment:18 by jscipione, 4 years ago

Aha! It’s probably looking for the translated name in Appearance but in app server after a reboot it is looking for “Default” without translating. So that’s why it works for me in English but not for you. It will be difficult to get the translated name in app server since it doesn’t have access to Appearances cat keys file, so we may have to look at the untranslated name in Appearance for it to match. Unfortunately... looking at the pop up menu label might not work in that case ... hmmm...

comment:19 by jscipione, 4 years ago

Actually... thinking about this a bit more... we should change the path of the Default decorator from "Default" to "" (i.e. a blank string) like what we are already doing for the control looks. That's why the control look pop up menu works in non-English and decorator doesn't. By using "" as the path of the default decorator instead of "Default" we localize it without having to worry about what the translation is. Then when the path is detected as "" in Appearance, we select the Default decorator, whatever that is called in your locale and everyone will be happy.

comment:20 by Starcrasher, 4 years ago

The default decorator is also working before a setting file exists therefore "" is probably already working and deleting decorator_settings should also work.
But decorator changes should be immediate and there's no effect before a reboot if you delete the file.
Maybe the app server understand "" at start up but needs you to send "Default" to update when it's running? Control Look settings changes are not live, could it be why they don't need that?

comment:21 by Starcrasher, 4 years ago

Again, I'm not a developer my logic can be wrong but from what I understand:
It works at boot with no settings file only because then app server initializes with its default value.
Since decorator changes are live, it means that windows are constantly receiving the info about decorator settings, right?
In this case, sending a path means 'uses this path starting from now' and sending "Default" 'reset the path'. But you probably don't want to send full path to each window constantly so when there's no change maybe are you sending nothing which is like "".
So using "" might not work.

by cafeina, 3 years ago

Attachment: bug.mkv added

Video capture of the bug with a Spanish language locale system.

comment:22 by cafeina, 3 years ago

I had the same problem with Spanish localisation. It's still present in hrev55185 and hrev55161, x86-64 platform.

With a freshly installed system, if you click the "About" button of the default decorator, it does not spawn the dialog box. It allows you to change to the "Be" decorator (the UI is updated), but when you choose the "Default" decoration back ("Predeterminado" as shown in Spanish) it does not change the decorator (even if the "Default" decorator in dropdown list becomes checked). The only way to get back the default decorator is to use the reset button. Then the decoration (with all of the Appeareance configs) resets to defaults and the "About" dialog box begins to work again.

After that, I changed the system language to English, and with a quick test everything works as intended, without workarounds.

comment:23 by jscipione, 3 years ago

I see what's going on. The problem only presents itself on non-English locale with a translated "Default" decorator which is why as a mono-lingual English speaking American I was not able to reproduce the problem on my system.

In app server [0] we look for the word "Default" to see if the Default decorator is loaded which is why it doesn't load the Default decorator in a non-English locale. It is difficult to translate the word "Default" in app server to compare against because we don't have access to the Appearance catalog there.

While it is possible to import the catalog from Appearance to get the translation for "Default", a better solution I think is to identify the Default decorator in app server using a blank string "" instead of using the string "Default". We're already doing this for ControlLook so we should copy this behavior for Decorator. By looking for a blank string instead of "Default" we sidestep the string comparison problem and if your settings file gets messed up app server will load the Default decorator.

[0] https://git.haiku-os.org/haiku/tree/src/servers/app/decorator/DecorManager.cpp#n279

comment:24 by cafeina, 3 years ago

Couldn't it search by some static identifier (whatever string it holds) instead of searching by the name string, so it can support translations?

comment:25 by jscipione, 3 years ago

Thinking about this a bit more, maybe we don't need to change app server after-all. We simply need to store the untranslated decorator name when saving to disk, at least for the default decorator.

The idea is to store the path to the decorator to disk, and paths are typically in English anyway. Since we look for the decorator in four places (system package, system non-package, user package, user non-package) we only store the leaf of the decorator path. We can still translate the "Default" string into your locale before adding the "Default" menu item to the menu field so that you see the localized version (e.g. Predeterminado) but we don't store that translated string to disk. Then app server will see "Default" on disk, and everything should be happy.

comment:26 by jscipione, 3 years ago

Should be fixed in https://review.haiku-os.org/c/haiku/+/4320 based on what I said. Save "Default" to disk instead of the localized Default decorator name. I still feel that using a blank string in app server would be better but this fixes the bug without having to change app server.

comment:27 by waddlesplash, 2 years ago

Milestone: UnscheduledR1/beta4
Resolution: fixed
Status: reopenedclosed

Fix merged in hrev55860.

Note: See TracTickets for help on using tickets.