Okay. Change submitted at
. This is an *
incremental* step, well aware that discussion on
is still in process. Once change
69336 is wrapped up, please do let us know and we'll modify
ZeroRatedMobileAccess accordingly - be that actually using GetMobileNotice
exclusively for banners as Max's code would suggest (would require
GetMobileNotice to pass additional context in order to support wiping out
the banners array to avoid Central Notice donation stuff from creeping back
into Wikipedia Zero contexts), continuing the current practice of
GetMobileNotice in one place and MinervaPreRender in another, or maybe even
having an altogether different hook (again, would need to be coordinated
with template backing fields).
-Adam
On Tue, Jul 2, 2013 at 11:05 AM, Jon Robson <jdlrobson(a)gmail.com> wrote:
Personally I don't see a reason. I'm not sure
about the history myself
but your assessment seems valid that it was probably introduced for
Zero.
We should remove it from SkinMobileWML as well.
If it's not documented kill it ;-)
On Tue, Jul 2, 2013 at 10:32 AM, Adam Baso <abaso(a)wikimedia.org> wrote:
MobileFrontend peeps:
Is it okay to remove the following line from SkinMobile.php, line 50?
wfRunHooks( 'GetMobileNotice', array( $this, &$notice ) );
Removal would reduce unnecessary double execution of the Wikipedia Zero
banner rendering code in HTML contexts.
Note that SkinMobileWML.php, line 34, will still call GetMobileNotice,
but I
don't think we need the invocation from
SkinMobile.php, line 50, unless
some
other component will be using it, such as
CentralNotice. I get the
impression that CentralNotice doesn't actually use GetMobileNotice, given
the following code on line 75 of SkinMinerva.php:
if ( $wgMFEnableSiteNotice ) {
$banners[] = '<div id="siteNotice"></div>';
}
My understanding is that Central Notice will update the <div
id="siteNotice"> with JavaScript when CentralNotice is turned on, and
that
Central Notice doesn't actually implement
GetMobileNotice (the git
history
suggests GetMobileNotice was coded for
generalized banners, but with a
very
specific callee, ZeroRatedMobileAccess, last
year) so it should be safe
to
remove from SkinMobile.php:50.
Okay to proceed with removal of the line?
Thanks.
-Adam
_______________________________________________
Mobile-l mailing list
Mobile-l(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mobile-l
--
Jon Robson
http://jonrobson.me.uk
@rakugojon