Clicking "Mobile view" in the footer, e.g. https://m.mediawiki.org/w/index.php?title=Extension:GuidedTour&mobileaction=toggle_view_mobile from https://www.mediawiki.org/wiki/Extension:GuidedTour gives
MediaWiki internal error. Exception caught inside exception handler. Set $wgShowExceptionDetails = true; at the bottom of LocalSettings.php to show detailed debugging information.
Strange. @Quiddity, do you see this with other MediaWiki.org Extension: pages or other pages?
I just tried going from https://www.mediawiki.org/wiki/Extension:ParserFunctions to https://m.mediawiki.org/w/index.php?title=Extension:ParserFunctions&mobileaction=toggle_view_mobile and the redirect worked, and went from desktop enwiki Main page with the footer link and those looked okay, too.
The problem appears to be in the brace expansion routine in the parser, wherein a sub-request is made to a special page in order to fetch its content. If configured, MobileFrontend injects some additional content into special pages, the routine for which will execute for the sub-request dispatched by the parser. Part of that routine involves getting the URL of the current request, which fails as one isn't set by default.
This appears to be a bug in the parser rather than a bug in MobileFrontend.
Here's a test page I've made that reproduces the bug: https://www.mediawiki.org/wiki/User:Phuedx_(WMF)/T129600. It's one line long:
{{Special:PrefixIndex/Extension:GuidedTour/}}@dr0ptp4kt That'd be wise. We can submit a change for review to broaden the discussion too.
@ssastry, @dr0ptp4kt: This bug can both be fixed in MobileFrontend and make MobileContext::doToggling simpler too. Change incoming…
Change 277969 had a related patch set uploaded (by Phuedx):
Don't parse URL to manipulate query parameters
Florian seems to have merged but please see
https://gerrit.wikimedia.org/r/#/c/277969/2/tests/browser/features/switch_views.feature
Let's close the discussions there before signing off.
@Jdlrobson I've responded to your first comment. I'm not inclined to mark the tests as integration tests as the switching steps should actually replace one of those damned cookie setting steps! I'll submit a follow-up change.
Looks like you fixed the problem with your changes in MobileFrontend.
I think you are suggesting that the bug in the parser is that the parser doesn't set a request URL on the FauxRequest .. but, looks like in all the uses of FauxRequest in core, only php unit sets a url on this object. It seems like it would be good to set url on the FauxRequest at least in the parser since otherwise the parser is leaking abstraction about the FauxRequest into hooks on Special Pages (hooks would have to know that Special Page parse is using a FauxRequest that might not be setting the request url). Maybe @Anomie, @Legoktm or @tstarling can say more about this.
I agree. The line in MobileFrontend that was highlighting the problem has been removed both because it highlighted the issue and because it was unnecessary. Indeed, the majority of 277969 is concerned with adding missing integration tests.
I'm happy to submit a patch to core to set the URL of the sub-request to fix the issue at its root.
Further to your point about leaky abstractions, I'd add that FauxRequest#getRequestURL throwing an exception which isn't thrown by WebRequest#getRequestURL is an LSP violation – and it's an undocumented exception too!
since otherwise the parser is leaking abstraction about the FauxRequest into hooks on Special Pages (hooks would have to know that Special Page parse is using a FauxRequest that might not be setting the request url)
Well, not necessarily. We could just document for those hooks that it's only valid to access the query parameters (->getVal() and so on) out of the request.
One question with setting a URL on the FauxRequest is whether that URL should be the actual URL of the request, or a faked-up URL for the special page. Another question is whether you also want to copy over the cookies, the Session, the headers, the IP, and so on (e.g. change to a DerivativeRequest).
https://gerrit.wikimedia.org/r/278416 will help in the mean time with the browser tests @phuedx
Change 278715 had a related patch set uploaded (by Phuedx):
Revert "Don't parse URL to manipulate query parameters"
Change 278715 merged by jenkins-bot:
Revert "Don't parse URL to manipulate query parameters"
Change 278729 had a related patch set uploaded (by Phuedx):
Don't parse URL to manipulate query parameters
Preferably, a faked up URL for the special page. The alternative, dispatching a request to a special page despite the URL not being for the special page. doesn't make sense – neither does dispatching a request to a special page without a URL…
It's likely that I'm misunderstanding but, depending on the special page being transcluded, switching to DerivativeRequest might vary the parser output depending on if the user is logged in at parse time. If we were to switch to DerivativeRequest, then shouldn't it be for a logged out user?
N.B. that the build of 278729 is currently failing due to an unrelated error from somewhere within #MediaWiki-extensions-WikibaseClient.
@dr0ptp4kt http://reading-web-staging.wmflabs.org/wiki/T129600 is a copy of the page that I could reproduce the bug from my local environment.
I want to make sure that mediawiki.org is no longer erroring and polluting logs before signing off.