|
Description
Martin Renvoize (ashimema)
2023-09-14 15:14:01 UTC
The issue I see here is that our required cookies don't require consent - they cannot be avoided if you want to use the OPAC. I am not sure how that would fit the modal. Just because they're required for operation doesn't mean we should hide them. We should have encouraged best practice which is to inform the user what the required cookies are used for.. That way then can then choose to opt out of using your service if they believe the required cookies are over the top. I'm all for informing users! Even if all the cookies are required and there is no choice for the user, we have to tell them what's happening. Can we slightly shift the cookie consent feature so that it's not just about the consent but mostly about the cookies? Or do we need a default Opac page about required cookies, with a link from the Opac footer? (Just throwing ideas wildly here.) Created attachment 159089 [details] [review] Bug 34793: Add default HTML Customisations for cookie information This patch adds some default html customisation content for the cookie consent feature. Test plan 1) Run through the installer and pick to import the sample news items 2) Confirm that after enabling the CookieConsent preference that the default content appears in the cookie bar at the bottom of the page and in the modal that appears after clicking 'More information' Mentored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 159090 [details] [review] Bug 34793: (follow-up) Rename 'Cookie consents' to 'Cookies policy' The modal is not always about consent, but rather about displaying the libraries cookie policy. It makes sense to update the heading to reflect that. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Aude did all the hard work here putting together the text, I just helped to construct the patch and upload it. Great work Aude, I think it looks much better now as a feature. Created attachment 159094 [details] [review] Bug 34793: Add default HTML Customisations for cookie information This patch adds some default html customisation content for the cookie consent feature. Test plan 1) Run through the installer and pick to import the sample news items 2) Confirm that after enabling the CookieConsent preference that the default content appears in the cookie bar at the bottom of the page and in the modal that appears after clicking 'More information' Mentored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 159095 [details] [review] Bug 34793: (follow-up) Rename 'Cookie consents' to 'Cookies policy' The modal is not always about consent, but rather about displaying the libraries cookie policy. It makes sense to update the heading to reflect that. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Quick typo fix applied ;P I've tested and am happy to sign off. Comments/suggestions: 1. Should the cookie bar link to the cookie policy? I only see this in the OPAC by going to Your account > Consents > under Cookies policy is "View your cookie consents" button > pop up window has "Accept all cookies" irrespective of whether you have clicked on the "Accept all cookies" in the cookie bar. 2. The table in the Consent policy page in the OPAC. It looks fine when you preview in Tools. However, in the OPAC it has hard to read as the rows are aligned to the center, with no alternate row colours or border between rows. Can the table rows be aligned to the top, and formatted similar to the staff interface tools preview? 3. I'm not sure that I understand the normal workflow for this from a patron perspective, even after reading through bug 27378, and how it works with the privacy policy. Testing notes: 1. Starting the web installer in KTD: https://gitlab.com/-/snippets/1893788#user-content-starting-the-web-installer (In reply to David Nind from comment #10) > I've tested and am happy to sign off. > > Comments/suggestions: > > 1. Should the cookie bar link to the cookie policy? I only see this in the > OPAC by going to Your account > Consents > under Cookies policy is "View > your cookie consents" button > pop up window has "Accept all cookies" > irrespective of whether you have clicked on the "Accept all cookies" in the > cookie bar. Right now it depends.. but follow the tree here and you'll find we fixed that too in one of the next bugs ;P > 2. The table in the Consent policy page in the OPAC. It looks fine when you > preview in Tools. However, in the OPAC it has hard to read as the rows are > aligned to the center, with no alternate row colours or border between rows. > Can the table rows be aligned to the top, and formatted similar to the staff > interface tools preview? Looks like we missed the classes (annoyingly, the preview in staff always uses the staff css.. we should fix that somehow really) > 3. I'm not sure that I understand the normal workflow for this from a patron > perspective, even after reading through bug 27378, and how it works with the > privacy policy. It's in the documentation list.. hopefully aude and I can work through it asap. > Testing notes: > > 1. Starting the web installer in KTD: > https://gitlab.com/-/snippets/1893788#user-content-starting-the-web-installer Created attachment 159132 [details] [review] Bug 34793: (follow-up) Add table classes to popup This patch adds the appropriate table classes to the Popup html customisation block. This ensure we have proper table display formatting in the modal on the OPAC. PS, I set up a sandboxes with the full set of bugs applied if, it includes the follow-up I just added here too. http://opac-cookies.sandboxes.ptfs-europe.co.uk Created attachment 159133 [details] [review] Bug 34793: Add default HTML Customisations for cookie information This patch adds some default html customisation content for the cookie consent feature. Test plan 1) Run through the installer and pick to import the sample news items 2) Confirm that after enabling the CookieConsent preference that the default content appears in the cookie bar at the bottom of the page and in the modal that appears after clicking 'More information' Mentored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 159134 [details] [review] Bug 34793: (follow-up) Rename 'Cookie consents' to 'Cookies policy' The modal is not always about consent, but rather about displaying the libraries cookie policy. It makes sense to update the heading to reflect that. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 159135 [details] [review] Bug 34793: (follow-up) Add table classes to popup This patch adds the appropriate table classes to the Popup html customisation block. This ensure we have proper table display formatting in the modal on the OPAC. Signed-off-by: David Nind <david@davidnind.com> (In reply to Martin Renvoize from comment #13) > PS, I set up a sandboxes with the full set of bugs applied if, it includes > the follow-up I just added here too. > > http://opac-cookies.sandboxes.ptfs-europe.co.uk Thanks Martin! I had missed the other bugs. I can see the bigger picture now... Thanks also for fixing the CSS for the OPAC display. Considering this series is a set that really aught to have been included in 23.11.00.. I'm a little disappointed they're still stuck waiting for QA :( Created attachment 162014 [details] [review] Bug 34793: (QA follow-up): forgotten 'Cookie consents'=>'Cookies policy' > The online catalog uses functional cookies: these are essential to make the site work.
It says the same on the staff interface >_<
Nothing that would work for both comes to my mind, any ideas?
Or do we still call it online catalog in the staff interface and whatever.
And like if for some libraries there is really a difference in cookies in the staff and OPAC because of some staff plugin or additional tracking of librarians (boooo!). Then they would submit/order a patch to have two different additional content entries for staff and OPAC and that will also solve this.
Anyway, most important is if there would be a nice phrasing instead of "online catalog" that would also work for the staff. Maybe not something as generic as "this web application".
Before that stuff is half set in stone when it's shipped and in the DB of instances.
---
There doesn't seem to be other relevant occurrences after searching "Cookie consent" and "consent" (case unsensitively). Some stuff is still user facing but it's for the librarians managing html_customizations and that would require changing existing data so not worth it.
---
Maybe a follow-up ticket is needed: to have these ticked by default in the installer. Because "sample news items" looks like demo stuff and not something that every library in EU and Australia (at least) needs to have. So they would miss it. If it's by default, when enabling the syspref, the English default text would immediately pop so they can directly use it or translate it without having to search for the list of cookies Koha use for example.
Oh wait, shouldn't it even be in a database upgrade instead of installer's optional sample data? (then it's a matter for this ticket) For existing instances that enabled the consent but don't have the additional content entries related to cookies policy. Like due to not immediately knowing what to put there (cookie list and explanation what are cookies).
It's also a way to address the above paragraph.
---
I though I was close to having finished reviewing without having found issues >_<
And this is why bug sets need to go in as sets! I don't think we need to change the text, yes the whole feature should be at interface level but it's not. And that's a big job compared to polishing the polishing this bug achieves. This sort of content change at upgrade isn't generally something we do. It risks breaking existing setups for little win.. and the sooner this goes in the less such an upgrade is needed. Thank you for looking into our cookie recipes Victor! ;-) Yes, we are fully aware the default text for CookieConsentBar specifically references the catalog... even though it will also be seen on the staff interface. Ideally, there should be separate HTML customizations for the Opac cookies and the Intranet cookies. As you pointed out, the cookies used on the two sites could be different. We discussed this with Martin back in September and we agreed the separate HTML customizations would be logged as follow-up bugs. Right now, we want to focus on making the Opac cookies work in a more logical way and on helping the library team get a head start when using the feature. We assume the public-facing side is more of a priority for library teams. I don't think the wording "the online catalog" needs to be changed. Here's why: 1. When creating a new HTML customization and selecting a display location, CookieConsentBar and CookieConsentPopup are under the Opac section. This makes it clear to library staff the default text is aimed at the Opac. 2. It's default text: by definition, it's meant to be built on. I expect library teams to amend the wording to their liking. 3. The essential cookies described in CookieConsentPopup are only those used on the Opac. According to the wiki (https://wiki.koha-community.org/wiki/Use_of_Cookies) there are more in the intranet. Having both sets in the HTML customizations would make it messy and confusing, which completely undermines the concept of proposing a clear and simple default cookie text - that we're trying to achieve here. Sorry it turned into a long reply! I think Martin covered the more technical questions :-) > I don't think we need to change the text ok, it was in case a native speaker and closer to the domain than me had an idea. Thanks Aude for the confirmation. No problem here. > yes the whole feature should be at interface level but it's not. And that's a big job compared to polishing the polishing this bug achieves. Totally. > This sort of content change at upgrade isn't generally something we do. It risks breaking existing setups for little win.. So like existing setups for which having the currently empty CookieConsentBar or CookieConsentPopup being filled would be a problem? : then add the two html customisation only if CookieConsent is disabled. The current proposal is only usable for new installs that would guess that "sample news items" in the installer might have interesting stuff for production. And those who use CookieConsent will have the pleasant surprise to get that nice prewritten cookies policy. Or have the very specific knowledge that there is the sample cookie policy there. The current win is little. If there is no simple immediately consensual way to make the current text available outside of the current narrow cases, then let's see the current proposal as an incremental step. This still has value as demo data and for people who know how to find it for new installs. Created attachment 162039 [details] [review] Bug 34793: Add default HTML Customisations for cookie information This patch adds some default html customisation content for the cookie consent feature. Test plan 1) Run through the installer and pick to import the sample news items 2) Confirm that after enabling the CookieConsent preference that the default content appears in the cookie bar at the bottom of the page and in the modal that appears after clicking 'More information' Mentored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 162040 [details] [review] Bug 34793: (follow-up) Rename 'Cookie consents' to 'Cookies policy' The modal is not always about consent, but rather about displaying the libraries cookie policy. It makes sense to update the heading to reflect that. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 162041 [details] [review] Bug 34793: (follow-up) Add table classes to popup This patch adds the appropriate table classes to the Popup html customisation block. This ensure we have proper table display formatting in the modal on the OPAC. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 162042 [details] [review] Bug 34793: (QA follow-up): forgotten 'Cookie consents'=>'Cookies policy' Works, makes sense, QA script happy, code looks good, passing QA :) Just stumbled on this one a little bit: Cookies policy. It looks like Cookie policy might be a little more common, but relying on the native speakers there. Pushed for 24.05! Well done everyone, thank you! Not backported to 23.11.x |