Bug 390330 - Calendar top column does not contain all the days. Only Sat, Sun
Summary: Calendar top column does not contain all the days. Only Sat, Sun
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Calendar widget (show other bugs)
Version: 5.8.7
Platform: openSUSE Linux
: NOR normal
Target Milestone: 1.0
Assignee: Plasma Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-12 17:09 UTC by Ahmad El-Gazzaz
Modified: 2019-01-03 22:03 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Screenshot of the calendar not showing all the days in the top column. (25.60 KB, image/png)
2018-02-12 17:09 UTC, Ahmad El-Gazzaz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad El-Gazzaz 2018-02-12 17:09:06 UTC
Created attachment 110568 [details]
Screenshot of the calendar not showing all the days in the top column.

I changed the start of the week to Saturday and the days at the top columns viewed only Sat, Sun. the rest is blank.

I don't recall how I changed the start of the week to Saturday.

I am running openSUSE 42.3
KDE Plasma: 5.8.7
KDE Frameworks: 5.32.0
QT: 5.6.2
Kernel: 4.4.87-25-default
OS Type: 64-bit
Comment 1 Kai Uwe Broulik 2018-02-13 08:48:37 UTC
> I changed the start of the week to Saturday

Can you explain to me how you did that so I can reproduce your problem? I bet it's just not wrapping the days properly, ie. it starts with 6 for Saturday, goes to 7 for Sunday, and then fails to wrap around and uses 8 which it obviously doesn't know.
Comment 2 Ahmad El-Gazzaz 2018-02-14 06:50:06 UTC
I edited the file: /usr/lib64/qt5/qml/org/kde/plasma/calendar/MonthView.qml

The commented line is the Original and the one I edited is below it.

Calendar {
        id: calendarBackend

        days: 7
        weeks: 6
        //firstDayOfWeek: Qt.locale().firstDayOfWeek    
        firstDayOfWeek: 6
        today: root.today

        Component.onCompleted: {
            daysModel.setPluginsManager(EventPluginsManager);
        }

        onYearChanged: {
            updateYearOverview()
            updateDecadeOverview()
        }
    }
Comment 3 Chris Holland 2018-09-05 18:38:45 UTC
This is because DaysCalendar.qml uses:
https://github.com/KDE/plasma-framework/blob/8497832a8bf088a9011052841a50863ab40bf062/src/declarativeimports/calendar/qml/DaysCalendar.qml#L315

Assumes firstDayOfWeek is 0 (aka Sunday). Using firstDayOfWeek=6 (Saturday) breaks this logic.

    text: Qt.locale(Qt.locale().uiLanguages[0]).dayName(calendarBackend.firstDayOfWeek + index, Locale.ShortFormat)

instead of:
    calendarBackend.firstDayOfWeek + index
we need to use:
    (calendarBackend.firstDayOfWeek + index) % 7

I'm assuming the number of days in a week doesn't change (7), though I'm not sure if there's a variable we can use to substitute.
http://doc.qt.io/qt-5/qml-qtqml-locale.html#weekDays-prop

You should be able to modify this locally, then restart plasmashell, or wait for a patch.
/usr/lib/x86_64-linux-gnu/qt5/qml/org/kde/plasma/calendar/DaysCalendar.qml

I'm not sure if locales that start on Saturday or Monday also experience this. Gonna check.
Comment 4 Chris Holland 2018-09-06 01:54:56 UTC
Ooooh, now I get why values of Sunday=0, and Monday=1 work, but Saturday=6 does not.

The first screenshot show Satuday=6 and Sunday=7 fine since Qt converts Sunday=0 to 7 anyways.

(Links to Qt code if you're bored, skip otherwise)
https://github.com/qt/qtdeclarative/blob/5.11/src/qml/qml/qqmllocale.cpp#L608
https://github.com/qt/qtbase/blob/5.11/src/corelib/tools/qlocale.cpp#L2517

By default the firstDayOfWeek is Sunday=0 for me (en_CA), but if we change it to (sv_SE) where firstDayOfWeek is Monday=1, it still shows up fine (with Sunday=7).

plasmoidviewer -a org.kde.plasma.calendar
LC_TIME="sv_SE.UTF-8" plasmoidviewer -a org.kde.plasma.calendar

I don't think *any* Locale uses a first day of the week other than Sunday or Monday, which is why this bug is only noticed when we hardcode the QML.
Comment 5 Ahmad El-Gazzaz 2018-09-06 06:09:57 UTC
(In reply to Chris Holland from comment #3)
> You should be able to modify this locally, then restart plasmashell, or wait
> for a patch.
> /usr/lib/x86_64-linux-gnu/qt5/qml/org/kde/plasma/calendar/DaysCalendar.qml


I tried to edit the file as you suggested the widgets failed.
The line I changed was 315. The commented line is the original and the one I edit is below
//                 text: Qt.locale(Qt.locale().uiLanguages[0]).dayName(calendarBackend.firstDayOfWeek + index, Locale.ShortFormat)
                text: Qt.locale(Qt.locale().uiLanguages[0]).dayName(calendarBackend.firstDayOfWeek + index) % 7, Locale.ShortFormat)

And this is the result I got from the widgets.


Error loading QML file: file:///usr/share/plasma/plasmoids/org.kde.plasma.digitalclock/contents/ui/main.qml:54:34: Type CalendarView unavailable
file:///usr/share/plasma/plasmoids/org.kde.plasma.digitalclock/contents/ui/CalendarView.qml:352:9: Type PlasmaCalendar.MonthView unavailable
file:///usr/lib/x86_64-linux-gnu/qt5/qml/org/kde/plasma/calendar/MonthView.qml:222:22: Type DaysCalendar unavailable
file:///usr/lib/x86_64-linux-gnu/qt5/qml/org/kde/plasma/calendar/DaysCalendar.qml:316:132: Expected token `,'


__________________________________________

(In reply to Chris Holland from comment #4)
> I don't think *any* Locale uses a first day of the week other than Sunday or
> Monday, which is why this bug is only noticed when we hardcode the QML.

Just change the locale to Egypt [مصر - العربية (ar_EG)] and many countries in the region and the start of the week will be Saturday.

Changing the local from systemsettings will results in too many changes across KDE that I do not want. Since I can no longer change every minor detail in the Regional settings I had to edit files directly to achieve what I want.

https://postimg.cc/image/6h3dl5pm9/
Comment 6 Chris Holland 2018-09-06 06:18:32 UTC
You're missing the second opening parentheses in `dayName((calendarBackend`.

Qt.locale(Qt.locale().uiLanguages[0]).dayName((calendarBackend.firstDayOfWeek + index) % 7, Locale.ShortFormat)

------

Ah, yes, I can confirm that the following locale reproduces the bug.

LC_TIME="ar_EG.UTF-8" plasmoidviewer -a org.kde.plasma.digitalclock

I recently added a "first day of the week" option to Event Calendar, I'll look into patching the digitalclock and calendar widgets.
Comment 7 Kai Uwe Broulik 2018-09-06 10:42:23 UTC
Git commit 020d3e3ebea595fc04642b5ec8f073f5a9478475 by Kai Uwe Broulik.
Committed on 06/09/2018 at 10:41.
Pushed by broulik into branch 'master'.

[Calendar] Wrap day name index around

Otherwise when the week starts on a day other than Sunday or Monday we access invalid day names.

CHANGELOG: Fixed week names not showing properly in calendar when week starts with a day other than Monday or Sunday

Differential Revision: https://phabricator.kde.org/D15309

M  +1    -1    src/declarativeimports/calendar/qml/DaysCalendar.qml

https://commits.kde.org/plasma-framework/020d3e3ebea595fc04642b5ec8f073f5a9478475
Comment 8 Chris Holland 2019-01-03 22:03:51 UTC
Git commit e5949866bb8f019a416930e2eb58bea363ff0ebf by Chris Holland.
Committed on 03/01/2019 at 21:58.
Pushed by cholland into branch 'master'.

[Calendar] Expose firstDayOfWeek in MonthView

This allows for calendar widgets to override the Locale. Users may
want to start the week on a Sunday, Saturday, or Monday without
changing their locale's date formatting.

Differential Revision: https://phabricator.kde.org/D15321

M  +1    -0    src/declarativeimports/calendar/qml/MonthView.qml

https://commits.kde.org/plasma-framework/e5949866bb8f019a416930e2eb58bea363ff0ebf