Page MenuHomePhabricator

$wgMFEnableSiteNotice always gets empty siteNotice
Closed, ResolvedPublic

Description

In SkinMinerva.php,

if ( $wgMFEnableSiteNotice ) {

$banners[] = '<div id="siteNotice"></div>';

}

forgets to put in the sitenotice there in the middle!!

$wgMFEnableSiteNotice = true;
$ GET -P '.../index.php?title=Raucously_Calling_for_War&useformat=desktop'|grep -i sitenotice

<div id="siteNotice"><div id="localNotice" lang="en" dir="ltr"><p>(server: jidanni2)

$ GET -P '.../index.php?title=Raucously_Calling_for_War&useformat=mobile' |grep -i sitenotice

<div id="siteNotice"></div>                             <div class="header">

$wgMFEnableSiteNotice = false;
$ GET -P '.../index.php?title=Raucously_Calling_for_War&useformat=mobile' |grep -i sitenotice
$ GET -P '.../index.php?title=Raucously_Calling_for_War&useformat=desktop'|grep -i sitenotice

<div id="siteNotice"><div id="localNotice" lang="en" dir="ltr"><p>(server: jidanni2)

Version: unspecified
Severity: normal

Details

Reference
bz63457

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:09 AM
bzimport set Reference to bz63457.
bzimport added a subscriber: Unknown Object (MLST).
Jidanni created this task.Apr 3 2014, 2:47 AM

bingle-admin wrote:

Prioritization and scheduling of this bug is tracked on Mingle card https://wikimedia.mingle.thoughtworks.com/projects/mobile/cards/1894

The site notice gets populated client-side when that div is present. One of the front-end engineers can confirm, but I think this is as expected and by design.

Please do this simple test in your LocalSettings.php:
$wgSiteNotice="BLA";
$wgMFEnableSiteNotice = true;
$ GET http://your_wiki/AnyPage?useformat=desktop|grep -i sitenotice

<div id="siteNotice"><div id="localNotice" lang="en" dir="ltr"><p>BLA

$ GET http://your_wiki/AnyPage?useformat=mobile |grep -i sitenotice

<div id="siteNotice"></div>

Please append your results below
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv

OR paste a screen show showing how the word BLA will appear in mobile.

Ah, I see what you mean. The <div id="siteNotice"></div> is used for CentralNotice notices, not to populate the value of $wgSiteNotice.

The only reason this exists is to support the CentralNotice extension so it can render banners. We should probably review this to see exactly what CentralNotice requires. We don't knowingly support $wgSiteNotice but maybe we should if it is supported in core.

Part of the problem here is one of nomenclature. EG we use the term 'siteNotice' when really we're talking about centralnotice notices, not the siteNotice stuff provided by core.

Removing site notices from a site causes legal issues. I highly recommend not removing them.

I would hate to be the star of a RISKS Digest article about why some of the staff were not alerted about a B*MB THREAT AT 13:00 etc.

(In reply to Dan Jacobson from comment #8)

Removing site notices from a site causes legal issues. I highly recommend
not removing them.
I would hate to be the star of a RISKS Digest article about why some of the
staff were not alerted about a B*MB THREAT AT 13:00 etc.

Dan forgive me but this feels rather alarmist to me. Further, while I am trying to assume good faith here, this also feels somewhat threatening.

The WMF sites use CentralNotice (https://www.mediawiki.org/wiki/CentralNotice) for notices, which is why the current functionality exists in MobileFrontend in its current form. If wgSiteNotice support is crucial for you and your project, we're happy to help support you and others in contributing patches to such an end.

The site notice might be the most important words on the whole site.
It should not disappear for users of the mobile version.

Untested patch:

  • $banners[] = '<div id="siteNotice"></div>';

+ $banners[] = '<div id="siteNotice">$wgSiteNotice</div>';

I am not on my WikiMedia machine today so cannot test it.

I mean MediaWiki. Oops.

Maybe it's more complicated than that. See what they do with $wgSiteNotice in Skin.php. The proper way would also make a '<div id="localNotice" lang="en" dir="ltr"><p>' so I better leave it up to the pros.

Change 123893 had a related patch set uploaded by MaxSem:
Refactor site notice handling

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

Change 123893 merged by jenkins-bot:
Refactor site notice handling

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