https://bugzilla.wikimedia.org/show_bug.cgi?id=56257
--- Comment #10 from Bartosz Dziewoński <[email protected]> --- I had a little chat about this with Krinkle on #wikimedia-dev today. Basically, it looks like we'll go with the original approach, it just needs some fixes – and while we're at it, let's change the selector for logo from "#p-logo a" to something saner like ".mw-wiki-logo" (and add this class to the appropriate element in the skins themselves). Sorry, but I don't have much time for Wikimedia things today/tomorrow, so let me just paste the log here: <Krinkle> MatmaRex: What about the sitelogo thing? Why is extending the wikmodule a problem? <MatmaRex> well, we need to override isKnownEmpty(), which looks hacky to me, and might not be the most efficient thing ever in general <MatmaRex> and we need to combine logic for cache invalidation with the existing one <MatmaRex> both of these mean that we need to much internal knowledge of RLWikiModule than i'm comfortable with when subclassing it like this <MatmaRex> (unless i'm missing something obvious to you) <Krinkle> MatmaRex: overriding isKnownEmpty is fine, because it unconditionally adds the css rule. Unless you make that css rule optional, there is no need to implement any known-empty logic. <Krinkle> As for the cache invalidation, that's just parent::() and then return max( $parentMTime, wglogomtime ) <Krinkle> which you calculate using the value-hash-cache-timestamp logic <MatmaRex> we have no wglogomtime i'm aware of, unless we store the time we last noticed it changing <Krinkle> yes <MatmaRex> ugh. <Krinkle> that's what we do for all values we cache that don't have timestamps. <Krinkle> we do the same for the language globals in langdata module <MatmaRex> sounds unpleasant, but alright <MatmaRex> so we stick to adding the logo to ResourceLoaderSiteModule? <Krinkle> and definition timestamp, and git version hash, etc. there is no other way to calculate it without lots of spurious invalidations (e.g. you'd need an array of all files that can possibly affect this global, and then some, and hte max() of their files, which is terrible and doesn't even always work with git and wmf-config) <Krinkle> MatmaRex: Yep <MatmaRex> Krinkle: while we're at it, let's change "#p-logo a" to ".mw-logo", hm? <MatmaRex> .mw-wiki-logo, rather <Krinkle> MatmaRex: Be sure to document that this means we will now always add a request for modules=site&only=styles on every page, where previously this only happened on wikis that enabled sitejs/css and actually created 1 or more wiki pages <Krinkle> MatmaRex: Sounds good <MatmaRex> document where? <Krinkle> MatmaRex: Commit message <Krinkle> MatmaRex: Also, given the above, this will now depend on https://gerrit.wikimedia.org/r/#/c/95463/ <MatmaRex> ah, duh. sure <MatmaRex> hmm? <Krinkle> MatmaRex: If not, you need to change OutputPage to take these new factors into account, but probably easier to make it dependent on https://gerrit.wikimedia.org/r/#/c/95463/ <MatmaRex> why would it? it seemed to work when i tested very biriefly <Krinkle> because we need to load site module unconditionally <Krinkle> MatmaRex: Delete your local site js/css pages <Krinkle> and try again :) <MatmaRex> i have none. it loaded the site module when i changed isKnownEmpty to return false, i think <Krinkle> MatmaRex: OK, in addition, disable the wg variarbles that enable it. <Krinkle> They are false on a new wiki by default. <MatmaRex> i'mmm pretty sure they're true by default <MatmaRex> wgUseSiteJs / Css? <MatmaRex> or something <MatmaRex> ok anyway, will have to check this <Krinkle> MatmaRex: okay, they weren' always true by default. Whatever, the point is, OutputPage has some harccoded logic beyond isKnownEmpty that will break your change if you don't fix it. <MatmaRex> got it, thanks <Krinkle> Which is ugly, and why I remove it with https://gerrit.wikimedia.org/r/#/c/95463/ -- You are receiving this mail because: You are the assignee for the bug. You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
