Last modified: 2014-04-04 20:02:58 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T65457, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 63457 - $wgMFEnableSiteNotice always gets empty siteNotice
$wgMFEnableSiteNotice always gets empty siteNotice
Status: RESOLVED FIXED
Product: MobileFrontend
Classification: Unclassified
Feature requests (Other open bugs)
unspecified
All All
: Unprioritized normal
: ---
Assigned To: Nobody - You can work on this!
: easy
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-04-03 02:47 UTC by Dan Jacobson
Modified: 2014-04-04 20:02 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Dan Jacobson 2014-04-03 02:47:17 UTC
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)
Comment 1 Bingle 2014-04-03 02:50:08 UTC
Prioritization and scheduling of this bug is tracked on Mingle card https://wikimedia.mingle.thoughtworks.com/projects/mobile/cards/1894
Comment 2 Arthur Richards 2014-04-03 17:52:37 UTC
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.
Comment 3 Dan Jacobson 2014-04-03 18:09:15 UTC
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
Comment 4 Dan Jacobson 2014-04-03 18:10:37 UTC
OR paste a screen show showing how the word BLA will appear in mobile.
Comment 5 Arthur Richards 2014-04-03 18:13:52 UTC
Ah, I see what you mean. The <div id="siteNotice"></div> is used for CentralNotice notices, not to populate the value of $wgSiteNotice.
Comment 6 Jon 2014-04-03 18:57:01 UTC
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.
Comment 7 Arthur Richards 2014-04-03 18:59:23 UTC
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.
Comment 8 Dan Jacobson 2014-04-03 19:10:24 UTC
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.
Comment 9 Arthur Richards 2014-04-03 19:17:10 UTC
(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.
Comment 10 Dan Jacobson 2014-04-03 20:58:20 UTC
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>';
Comment 11 Dan Jacobson 2014-04-03 20:59:17 UTC
I am not on my WikiMedia machine today so cannot test it.
Comment 12 Dan Jacobson 2014-04-03 20:59:49 UTC
I mean MediaWiki. Oops.
Comment 13 Dan Jacobson 2014-04-03 23:14:29 UTC
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.
Comment 14 Gerrit Notification Bot 2014-04-04 18:07:49 UTC
Change 123893 had a related patch set uploaded by MaxSem:
Refactor site notice handling

https://gerrit.wikimedia.org/r/123893
Comment 15 Gerrit Notification Bot 2014-04-04 20:02:14 UTC
Change 123893 merged by jenkins-bot:
Refactor site notice handling

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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links