> The acceptance criteria says We will add test coverage for the Skin class but Skin.js has no coverage.
Many tests were added for lazy image loading but Skin itself is still not especially testable. I'll look into this.

@Jdlrobson, per your AC I've taken a closer look at Skin and I don't think it will be the best use of time to write tests for it. Part of the reason is that Skin does not provide many seams and it modifies the page's HTML. I'm thinking at the end of it we would have similar feelings as we did for CtaDrawer.test.js. It's only about 140 lines but look at what we're actually validating:

Last acceptance criteria says that we will remove usage of the jQuery lib, but the jQuery is still required for Deferreds and declaring types for function params, and attaching eventListeners.

jQuery is the underlying implementation for util.Deferred() which lazyImageLoader uses. lazyImageLoader has no direct jQuery dependency. Swapping out the underlying util implementation for Promises or a polyfill is out of scope.

This is an example configuration currently supported: https://www.mediawiki.org/wiki/Extension:QuickSurveys#Configuration.

onExtensionThanksLogEntryTagCreate

Sorry, coming into this discussion a little late. Can this be generalized to any tag and be a change in Core instead? Something like onExtensionLogEntryTagCreate()?

@AndyRussG, do you think it would be possible to check the headers in CentralNotice.hooks.php? When a banner header with a value of null is set, maybe we don't call $out->addModules( 'ext.centralNotice.startUp' );. This would disable the CentralNotice banner JavaScript from executing.

I don't know if this is the same issue or not (it goes back at least as far as StoryPops 30c9de1) but I see pretty bad double pokeys in the StoryPops "RTL thumbnails" section:

@Jdlrobson, I can reproduce my earlier findings on Ubuntu Chromium v71.0.3578.98:

We discussed this in super happy dev time. Removing myself as assignee.

This still seems to be an issue:

Thanks, @AndyRussG!! I'll assign this to @Peter on Thursday when the train arrives on enwiki.

@Jdlrobson, sorry for the hassle but I'm glad it's fixed

Thanks, @Peter! I'll assign this to you when the dependent patch, T215086, is merged!

I think this is a duplicate of T202934.

@Peter, we monitor the Facebook and Barack Obama pages on both desktop and mobile.

@Niedzielski do you know if there's a reason for that getTitle() does not return a valid title when there are other parameters in the link URL than title=?

I do not know. There's a test case for multiple query parameters but no explanation. Perhaps, it's to avoid showing previews for any usage of the action API. E.g., https://en.wikipedia.org/w/index.php?title=Barack_Obama&action=history.

This appears to be fixed on the beta cluster:

Note: drawer contents can scroll.

There's some discussion about whether it would be better to hide the tabs in CSS or never send the HTML in the first place. The benefit of the former is the change is made only to presentation. The benefit of the latter is no possible confusion for screen readers and improved bandwidth.

rootpage_MainPage could be used to distinguish the main page in CSS. There is also a main page stylesheet.

I submitted a few hygiene patches while analyzing this. In combination with T212376, I think these changes are adequate. The page issues code could still use some big improvements, but the low hanging fruit has been plucked. If there are no objections, I advise that the outstanding patches be considered to resolve both T212371 and T212376.

Added discuss bullet to next super happy dev time.

This is ready for sign off by anyone.

So it's my understanding that https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/485957/ and https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/485956/ are the outcomes of this spike and we're just waiting a write up/discussion of that experience. Is that correct?

Yes, the offsite would be a good venue for this.

@Jdlrobson, should I create a dependency, move the module consolidation requirements to T213340, make a new task, or remove the requirements?

I think this needs more work. mobile.editor is still present in extension.json and tests/qunit/mobile.editor.api/EditorGateway.test.js references mobile.editor.api.

Links that are part of the footnote, when clicked, open in a new tab

I'm not objecting to this requirement but rather asking about it. Is this a change in behavior from the way references work today? For me personally, I usually control or middle-click when a new tab is wanted. If this is a change, do want to introduce it as part of the initial reference previews deployment? If it was separate, it could be A/B tested. I don't have any strong opinions but wanted to ask to understand the thinking behind the requirement.

I've tagged an example patch that I hope is more illustrative. In my view, this is ready to work when priorities permit.

Over to @Jdlrobson for code review.