Opened 8 years ago

Closed 6 years ago

#7947 closed bug (fixed)

[Time] Calendar doesn't respect current locale's week start day

Reported by: hamish Owned by: zooey
Priority: normal Milestone: R1
Component: Preferences/Time & Date Version: R1/Development
Keywords: time locale Cc:
Blocked By: #6422, #7652 Blocking:
Has a Patch: no Platform: All

Description

The time preflet creates the calendar view with the default week-start day, Sunday.

The locale kit provides a function to retrieve the week-start day for the current locale.

Attachments (2)

time_weekday_27082011.patch (3.8 KB ) - added by hamish 8 years ago.
query locale kit for week start
calendar.png (16.4 KB ) - added by dsjonny 6 years ago.

Download all attachments as: .zip

Change History (25)

by hamish, 8 years ago

Attachment: time_weekday_27082011.patch added

query locale kit for week start

comment:1 by hamish, 8 years ago

Has a Patch: set

comment:2 by zooey, 8 years ago

Owner: changed from axeld to zooey
Status: newin-progress

comment:3 by zooey, 8 years ago

Resolution: fixed
Status: in-progressclosed

Applied in hrev42697, thanks.

comment:4 by zooey, 8 years ago

Resolution: fixed
Status: closedreopened

Reopening - Axel has pointed out that BCalendarView should use the correct week-start day by default.

comment:5 by hamish, 8 years ago

Perhaps it might be good to have the CalendarView use ICU's calendar class? (http://icu-project.org/apiref/icu4c/classCalendar.html)

It provides a number of interesting locale-specific parameters for calendars (week start, weekends, etc) and may make it possible to implement non-Gregorian calendars.

in reply to:  5 comment:6 by taos, 8 years ago

Replying to hamish:

Perhaps it might be good to have the CalendarView use ICU's calendar class? (http://icu-project.org/apiref/icu4c/classCalendar.html)

It provides a number of interesting locale-specific parameters for calendars (week start, weekends, etc) and may make it possible to implement non-Gregorian calendars.

Would this take care of #6422 (BCalendar should allow starting weeks on any day) and #7652 ([Deskbar] Calendar is not localized)?

comment:7 by hamish, 8 years ago

Yes on #6422.

#7652 is caused by BDate using strftime which is not localised. BDate could use ICU to get the day/month names instead (DateFormatSymbols class), though this would make BDate dependent upon liblocale.so.

in reply to:  5 comment:8 by axeld, 8 years ago

Replying to hamish:

Perhaps it might be good to have the CalendarView use ICU's calendar class? (http://icu-project.org/apiref/icu4c/classCalendar.html)

I would rather try to limit the direct uses of ICU as much as possible, and gate everything we need through liblocale.so.

comment:9 by hamish, 8 years ago

Yeah, I meant wrap the calendar class in the locale kit, and then use that wrapper in CalendarView.

comment:10 by pulkomandy, 8 years ago

Both BCalendar and BDate are new in Haiku, so it's not a problem to have them require the locale kit, or even be moved inside it (they are in the "shared" private kit right now, I think ?).

comment:11 by hamish, 8 years ago

Here's my ideas for solving this ticket + #6422 and #7652

  1. Remove B_WEEK_START_* enum from CalendarView.
  2. Add more general B_DAY_* and B_MONTH_* enums to BDate.
  3. Add functions to locale kit to get day/month names using ICU (in BLocale/BFormattingConventions/new class?).
  4. Have BDate use those functions instead of strftime.
  5. Have BLocale::StartOfWeek() return any day instead of just Mon/Sun (using new BDate enums).
  6. Add something like BLocale::MinDaysInFirstWeek(), which is the other locale-specific calendar parameter provided by ICU.
  7. Have CalendarView query the locale kit for those parameters by default (also, have option to override that behaviour).

in reply to:  11 comment:12 by zooey, 8 years ago

Status: reopenedin-progress

Replying to hamish:

Here's my ideas for solving this ticket + #6422 and #7652

  1. Remove B_WEEK_START_* enum from CalendarView.
  2. Add more general B_DAY_* and B_MONTH_* enums to BDate.
  3. Add functions to locale kit to get day/month names using ICU (in BLocale/BFormattingConventions/new class?).
  4. Have BDate use those functions instead of strftime.
  5. Have BLocale::StartOfWeek() return any day instead of just Mon/Sun (using new BDate enums).
  6. Add something like BLocale::MinDaysInFirstWeek(), which is the other locale-specific calendar parameter provided by ICU.
  7. Have CalendarView query the locale kit for those parameters by default (also, have option to override that behaviour).

All that sounds very reasonable and I have already implemented several parts of it locally, but I need to do some more work before I can commit (hopefully later today). Sorry that I didn't mark this ticket as 'in-progress' earlier, I should have.

What I find interesting is that strftime() doesn't seem to deliver the right month names, even when one changes the preflet to invoke setlocale() - all the locale-aware posix functions should work already, I must check why this isn't the case here ...

On top of what's listed above, I'd like to change BDate/BTime/BDateTime to use an ICU calendar as backend, since it doesn't really make sense to implement the stuff manually, when we have ICU. That would open support for other calendars than Gregorian, too.

For all of this to be possible, BDate/BTime/BDateTime & BCalendarView would need to use the locale kit. In order to avoid dependency problems between liblocale & libbe, I think we should just unite those two, i.e. make the locale kit a member of libbe.so.

comment:13 by zooey, 8 years ago

Ok, hrev42720 implements correct handling of week startdays - leaving this open until I know what goes wrong with strftime().

comment:14 by pulkomandy, 8 years ago

isn't BDateTime still part of BPrivate namespace ? If so, it could just be moved to liblocale, wthout having to merge it ?

Well, not that I'm against merging it, could make some things simpler and it's getting more and more tied up to other parts of the system...

comment:15 by axeld, 8 years ago

Since libroot.so uses liblocale.so as well AFAIK, I'm not sure this is going to work well, but merging it would have several benefits otherwise.

And yes, the BDate/BTime/BDateTime/whatever classes are experimental, and not yet finalized, so they could be moved/changed without problems if needed.

comment:16 by pulkomandy, 8 years ago

There is a bridge system to use locale classes from both libroot and libbe already (libroot for the posix date stuff, and libbe for localizing the color picker in interface kit). So moving it around shouldn't be too much of a problem ?

comment:17 by zooey, 8 years ago

To be precise: just libbe is using liblocale (via dlopen), libroot is dlopening a similar add-on, but that is not using liblocale, just ICU. So merging liblocale.so would allow us to get rid of the bridge mechanism in libbe without affecting libroot at all and make it much easier to add locale support to the libbe-classes.

In the long run, it doesn't make much sense to stuff everything into liblocale that has support for locales, as I suppose that would include most views (especially once we support right-to-left drawing of text).

comment:18 by axeld, 8 years ago

Sounds like the perfect solution indeed, then.

comment:19 by pulkomandy, 8 years ago

Blocked By: 6422 added

(In #6422) Duplicate of #7947 (which has more info).

comment:20 by pulkomandy, 8 years ago

Blocked By: 7652 added

(In #7652) Duplicate of #7947.

comment:6 by pulkomandy, 8 years ago

Has a Patch: unset

comment:7 by dsjonny, 6 years ago

Using hrev46706 the calendar looks correct (localized and the start day of the week is also OK) for hungarian locale:

by dsjonny, 6 years ago

Attachment: calendar.png added

comment:8 by pulkomandy, 6 years ago

Resolution: fixed
Status: in-progressclosed
Note: See TracTickets for help on using tickets.