Page MenuHomePhabricator

Talk icon doesn't show up for pages with Flow boards in alpha
Closed, ResolvedPublic

Description

Compare:
https://en.m.wikipedia.org/w/index.php?title=Wikipedia:WikiProject_Breakfast&mobileaction=alpha
...and...
https://en.m.wikipedia.org/w/index.php?title=Wikipedia:WikiProject_Breakfast&mobileaction=beta

Talk icon shows up in Beta mode, but not in Alpha mode.

I'm not actually sure which behavior is correct, but they should probably be the same.


Version: unspecified
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=67819

Details

Reference
bz71570

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:57 AM
bzimport set Reference to bz71570.

bingle-admin wrote:

Prioritization and scheduling of this bug is tracked on Trello card https://trello.com/c/q3UhD8Ur

Weird. It will be something to do with how wfRunHooks( 'SkinMinervaDefaultModules', array( $this, &$modules ) ); is run (which Flow uses).

Not sure what is happening here.

This seems to be because of how Alpha and stable recognize, if a talk button is useful or not. If the user has no JS enabled, the talk button should be hidden, if there is no talk page (the user can't create a page without JS in mobile)

For this there are these implementations:

Stable:
Check if the talk element has a new class:
https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/skins/SkinMinerva.php#L725
(with flow, there will be never a new class?!)

Alpha:
https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/skins/SkinMinervaAlpha.php#L74
Decide with the number of topics (which will be displayed in the talk icon). So, Flow never returns the number of topics, if i see this right, so the talk button will be hidden every time.

talk.js isn't loaded on pages with flow talk pages, too (?!), so the hidden class will never be removed, like it will be on "normal" talk pages :)

I guess we need to rethink this code. The fix should be in MobileFrontend I'm just not sure how best to do that.

Florian personally I'd remove the hidden class in alpha. It seems unnecessary and is wrong anyway ( some articles will have talk pages but no topic page prop value )

Hmm, i don't know, if this colidates with bug 67819. But it's more important, that a user can see any talk page, if there is one.

Change 165694 had a related patch set uploaded by Florianschmidtwelzow:
Revert partially: "Show talk button only when JS enabled or talk page not empty"

https://gerrit.wikimedia.org/r/165694

Change 165694 merged by jenkins-bot:
Revert partially: "Show talk button only when JS enabled or talk page not empty"

https://gerrit.wikimedia.org/r/165694

Doesn't actually seem to have fixed it on alpha.

(In reply to Jon from comment #9)

on alpha.

Because i left the alpha code (see commit message) as an alpha feature, not good?

Should remove it. Not sure why it needs to be hidden in alpha. As stated before, that page property value is not always set - especially with Flow pages :)

Change 165980 had a related patch set uploaded by Florianschmidtwelzow:
Revert "Show talk button only when JS enabled or talk page not empty"

https://gerrit.wikimedia.org/r/165980

Change 165980 merged by jenkins-bot:
Revert "Show talk button only when JS enabled or talk page not empty"

https://gerrit.wikimedia.org/r/165980