| dr0ptp4kt | |
| Aug 19 2016, 3:38 PM |
| F4451666: pasted_file | |
| Sep 9 2016, 6:39 PM |
| F4427152: Screenshot_2016-09-02-21-46-28.png | |
| Sep 2 2016, 8:52 PM |
| F4377013: borscht_w0_uh_oh.jpg | |
| Aug 19 2016, 3:38 PM |
| F4377011: borscht_no_w0_ok.jpg | |
| Aug 19 2016, 3:38 PM |
On Wikipedia Zero, the page actions at the top of the page are no longer showing. They should show so that Wikipedia Zero users are able to conveniently do things like switch language, watchlist pages, and edit pages.
Non-Wikipedia Zero:
Wikipedia Zero, cached, purged, and URL cache varied page; observed on both iOS Safari 9 and Firefox desktop:
At the end of this task we should have a recommendation on what to do next.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Jdlrobson | T144329 ZeroBanner / JSONConfig PHPUnit failure | |||
| Resolved | ovasileva | T143425 Page actions bar not showing on Wikipedia Zero |
This is a regression, as it used to be possible to see and use the edit pencil. I'm not sure when the regression started, though.
The original patch's purpose was reinstating the page actions bar, including the edit pencil, in case the operator zero-rated HTTPS.
Most operators used to not zero-rate HTTPS, but now all of them do. Although that sounds simple the logic through the tiers and this code is a little tricky.
The page actions are hidden at /modules/redux.less#L26 and are shown using JS at /modules/interstitial.js#L38. The test at /modules.interstitial.js#L37 is returning false, as a result of which, page actions stay hidden. The zerodontwarn setting is set at /modules/zerobanner.interstitial/ZeroOverlay.js#L49 when the user clicks on the Zero Overlay don't warn button. So basically, if the user clicks don't warn, we let the user see the page actions. I don't know why this was programmed that way. The solution is probably to show the page-actions on page load.
@jhobs / @dr0ptp4kt / @Yurik, any reason why page actions are hidden in the first place?
Change 307664 had a related patch set uploaded (by Jdlrobson):
Add basic test for Zero
I've written a basic test for this to show @bmansurov's patch fixes this and offer us more protection going forward. On the long term we should aim to pull zerobanner apart and get it 100% tested - there is lots of highly outdated code inside that module.
Progress with this patch is likely to be blocked until T144329 can be fixed.
@jhobs / @dr0ptp4kt / @Yurik, any reason why page actions are hidden in the first place?
Back in the day only contributory features required HTTPS. But most W0 operators didn't zero-rate HTTPS back then. However, when operators started adding support for zero-rating HTTPS, the code was structured in https://gerrit.wikimedia.org/r/#/c/113655/ to show the contributory feature icons if the operator zero-rated HTTPS (as well as if the user had at some point on an exit interstitial said don't warn me).
It appears that in https://gerrit.wikimedia.org/r/#/c/296175/4/modules/interstitial.js the check for supportsTLS was removed. @bmansurov, your analysis about the user having to have clicked the "don't warn me" button given that change is spot on. It's true that the page actions should inherit their default visibility from MobileFrontend at this point...although the most surgical fix is called for on this task so that other fine grained logic in the page doesn't break.
Thanks, @dr0ptp4kt.
Spike results:
https://gerrit.wikimedia.org/r/307664 should be merged before sign off and preferably this test should show up in MobileFrontend as a result of T144412.
Change 307826 had a related patch set uploaded (by Jdlrobson):
Move qunit tests to tests/qunit
Please do not sign off until T144412 is resolved and we can verify this test runs with MobileFrontend so this never happens again.
QUnit Failure can be seen here:
https://integration.wikimedia.org/ci/job/mwext-qunit-jessie/4015/consoleFull
I'll SWAT this today.
I'm not sure how to verify the page actions are showing again on Zero.
@dr0ptp4kt can you provide guidance?
Mentioned in SAL [2016-09-02T00:03:31Z] <dereckson@tin> Synchronized php-1.28.0-wmf.17/extensions/ZeroBanner: Update router code (T143425) (duration: 00m 47s)
@dr0ptp4kt: Is this something that you can email the Reading Web team about so that we're all capable of testing/signing off on these changes?
Sections are also uneditable now. Can you check if you have the same problem or it's a personal issue please.
I see the same problem. I don't think it's because of the changes we made recently. @Issimo_15 do you think we didn't have this problem up until recently?
Change 308802 had a related patch set uploaded (by Bmansurov):
Show edit icon and page actions
@dr0ptp4kt @ovasileva should the edit icon / page actions be not showing under any circumstances in Zero right now? Per Adam it sounds like this might be a mistake but it has not explicitly been asked for yet
@bmansurov patch makes them show unconditionally - is this what we want?
From my understanding, we should be displaying the page actions in Zero, @dr0ptp4kt, can you confirm?
Yes, nowadays, the page actions bar and its buttons should show in the same fashion for Wikipedia Zero users as for non-Wikipedia Zero users.
@ovasileva, it's a little complicated. I think @dr0ptp4kt needs to give you access to the Zero portal so that you can add you IP address there and test the change.
Just to update: @ovasileva and I have a meeting later today to go over how to test Zero tasks, so it should be able to be resolved after that.
@ovasileva I don't think the latest patch has been SWAT deployed yet. If we don't SWAT deploy, then you can see page actions in action on Thursday.
Ignore my previous comment; I hadn't read Baha's latest comment yet and thought the patch had already been deployed.
@jhobs could you swat the fix early tomorrow? I'm not setup to test Zero related changes at the current time and it would be great to wrap up this work.
Is this something that needs to be SWATted? I think this should just ride the train. We should avoid SWATs if not UBN or a config change (see: team norm).
@Jhernandez: Is this UBN? It does render two stable features – the editor and the language switcher – unusable.
Change 310601 had a related patch set uploaded (by Jhobs):
Display edit icon and page actions
And subsequently deployed. Can be tested on enwiki (e.g. https://en.m.wikipedia.org/wiki/Dave_Grohl, you may have to add ?action=purge depending on timing). Throwing over to @ovasileva for signoff!
Change 307662 abandoned by Jdlrobson:
Add basic test for Zero
Reason:
Abandoning old patch