Page MenuHomePhabricator

CookieWarning appears confusingly inside dismissable sitenotice
Closed, ResolvedPublic

Description

I tested this on MW 1.35.1 with the 48f795d version of CookieWarning (the latest in REL1_35 branch).

The HTML for CookieWarning appears inside the sitenotice, which then causes the sitenotice to appear even when there's no actual notice to be shown. This creates a confusing [dismiss] link at the top right of the page which hides both the sitenotice (if present at all) and the CookieWarning. That is really confusing to site admins and users.

One can see this bug also on Miraheze, e.g. here: https://meta.miraheze.org/
Related task: T254302: Update CookieWarning so it doesn't use the SkinTemplateOutputPageBeforeExec hook

Event Timeline

Restricted Application added subscribers: RhinosF1, Reception123, Aklapper. · View Herald Transcript

This is most likely a problem with the DismissableSiteNotice extension, rather than MediaWiki-extensions-CookieWarning. CookieWarning was changed to use the sitenotice functionality of MediaWiki, as it is the more correct place where such a notice should be added (see also the discussion in your linked issue). However, DismissableSiteNotice seems to does not work well with the changed styling of the cookieWarning, which, for Vector, is absolutely positioned at the top of the page.

Short solution would be: Uninstall DismissableSiteNotice.

However, there should probably a better solution, somehow. I, however, don't know how DismissableSiteNotice works internally, and what the intention really is. At the moment, there is only one "dismiss" link, even if there're multiple site notices (which a user could potentially dismiss independently as well). So maybe the solution is, that the extension is only adding such a link to site-notices that are actually rendered by the available message keys (Sitenotice and Anonnotice). Extension generated notices should probably be excluded from this.

Short solution would be: Uninstall DismissableSiteNotice.

Uhh, right. So the extension is being merged into core anyway (see T262118) and though it was just temporarily reverted because it caused some magical issues (T271365), the idea is the same as in the original extension. Basically, it checks whether the sitenotice is not empty and if so it wraps the notice's HTML into more HTML, including the dismiss button. There's no "multiple notices" system at all, it's a single string. And the cookie warning should not be dismissed using the notice system, that's just wrong.

CookieWarning was changed to use the sitenotice functionality of MediaWiki, as it is the more correct place where such a notice should be added (see also the discussion in your linked issue)

Why? Can't we just make it an absolutely-positioned thing on all skins, at the bottom of the page, like most of the Internet? Also keeping the banner at the top of the page on desktop skins is not optimal, as it obscures vital UI components such as the search box or the site's name on skins like Timeless and Vector.

Can't we just move this warning out of the sitenotice? It's not designed for this and there's no real need for the warning to be there, provided we don't stick to the current positioning of it on Minerva. While we are at it, why not unify the extension's JS/CSS and make it responsive, with a single RL module?

So, I'm really short on time for the moment, but I found some to at least partially tidy this up and deploy on my wiki, as I really don't wan't someone to shout at me for not displaying the warning when I should.

The code is on a fork here, and yes, I know it should be on Gerrit and have working tests, but I didn't have the time do this, sorry. I'll upstream it later, but no promises on the timeline :)

Also: has anyone checked how the current version of this displays on mobile on Miraheze? I stumbled upon it by accident, looks pretty bad if you ask me.

image.png (645×363 px, 45 KB)

Why? Can't we just make it an absolutely-positioned thing on all skins, at the bottom of the page, like most of the Internet? Also keeping the banner at the top of the page on desktop skins is not optimal, as it obscures vital UI components such as the search box or the site's name on skins like Timeless and Vector.

I agree with making CookieWarning absolute-positioned as skins handle sitenotice differently, most of the time the placement is not intuitive for a cookie banner as most people wouldn't expect it under the header and inside the content. Furthermore, with wikis that does not have any sitenotice, it causes a reflow in the content because of how sitenotices are placed.

Change 660775 had a related patch set uploaded (by Ostrzyciel; owner: Ostrzyciel):
[mediawiki/extensions/CookieWarning@master] Move the warning outside sitenotice, tidy up code

https://gerrit.wikimedia.org/r/660775

As promised in the patch, here are some screenshots of it on different skins and screen sizes.

cookiewarning 4.png (1×1 px, 202 KB)
cookiewarning 2.png (1×750 px, 194 KB)
cookiewarning 1.png (1×750 px, 227 KB)
cookiewarning 3.png (2×1 px, 380 KB)

Change 660775 merged by jenkins-bot:
[mediawiki/extensions/CookieWarning@master] Move the warning outside sitenotice, tidy up code

https://gerrit.wikimedia.org/r/660775

Ostrzyciel claimed this task.

Change 660803 had a related patch set uploaded (by Paladox; owner: Ostrzyciel):
[mediawiki/extensions/CookieWarning@REL1_35] Move the warning outside sitenotice, tidy up code

https://gerrit.wikimedia.org/r/660803

Change 660803 merged by Jack Phoenix:
[mediawiki/extensions/CookieWarning@REL1_35] Move the warning outside sitenotice, tidy up code

https://gerrit.wikimedia.org/r/660803