Page MenuHomePhabricator

Ensure discussiontools markup is hidden by CSS when parser split is removed
Closed, ResolvedPublic

Description

This task about making sure that once the parser cache split is removed, the DiscussionTools features visible to people align with the settings the wiki they are using has set as well as the settings they, as individuals, have set within Special:Preferences.

Requirements

Meta

  • This patch can be tested on the Beta Cluster

Feature visibility

  1. People who have the Enable quick replying setting ENABLED within #mw-prefsection-editing should see [ reply ] links on the talk pages they visit
  2. People who have the Enable topic subscription setting ENABLED within #mw-prefsection-editing should see [ subscribe ] links on the talk pages they visit
  3. People who have the yet-to-be-created Enable readability enhancements setting [i] ENABLED within #mw-prefsection-editing should see topic containers (T269950), new reply and topic affordances (T255560 + T267444) etc.
  4. People who have the Enable quick replying setting DISABLED within #mw-prefsection-editing should NOT see [ reply ] links on the talk pages they visit
  5. People who have the Enable topic subscription setting DISABLED within #mw-prefsection-editing should NOT see [ subscribe ] links on the talk pages they visit
  6. People who have the yet-to-be-created Enable readability enhancements setting [i] DISABLED within #mw-prefsection-editing should NOT see topic containers (T269950), new reply and topic affordances (T255560 + T267444) etc.
  7. Testing: Write a patch which turns off the parser cache split; with the split disabled and all Discussion Tool features disabled, we need to make sure that everything is properly hidden.

Meta
The "Feature visibility" requirements above should not be affected by whether an individual manually configured a particular setting or whether the wiki as a whole configured the setting in a particular way for all users.

Note: I've intentionally omitted the New Discussion Tool from the lists above considering the guidance @DLynch
shared in T285995#7195218: "Guiding principle: are there any visible changes inside the text of the talk page? The comment formatter will run."

Deployment

Deployment of this patch does NOT depend on the Performance and Data Persistence Teams considering it will not have any impact on the parser cache.

Done

  • Functionality is implemented that enables us to have the comment formatter run while ensuring the DiscussionTools features visible to people on talk pages align with the state of the settings they have set within Special:Preferences

i. The Enable readability enhancements setting will be implemented in T270316

Related Objects

Event Timeline

Background: we're going to be moving DiscussionTools out of beta on some wikis. When that happens, the parser-cache split will go away and the DT markup is going to be served to everyone, regardless of whether they have the features enabled. For people who have it disabled, we want to make sure that the reply/subscribe links are hidden.

I've updated the task description with requirements for this task.

@DLynch: do you see anything within the task description that is unexpected, missing, etc.?

@ppelberg the "deployment" section is sort-of irrelevant to this ticket -- it's fine to deploy the CSS-hiding that this references. It's not fine to deploy the removal of the parser cache split, but that's not covered by this ticket anyway.

@ppelberg the "deployment" section is sort-of irrelevant to this ticket -- it's fine to deploy the CSS-hiding that this references.

Noted. Task description updated.

It's not fine to deploy the removal of the parser cache split, but that's not covered by this ticket anyway.

I see. @DLynch, are you aware of a ticket that removes the parser cache split? If not, are you able to file a ticket for this work so I can mark it as a sub-task of T280599?

I ask the above thinking: we have T279864 and it seems like another ticket might be needed to disentangle the parser cache split from tools being offered by default.

I don't think we have an explicit ticket for removing the split, beyond it being kind of inherent in any ticket which is releasing the tool as opt-out.

I also currently think this task should just be a verification, rather than needing any implementation work. As far as I can see right now, any page which can possibly have the tool enabled should have the styles output.

If we decide to decouple the work of converging on. a single stored version talkpage HTML within the parser cache, from the work of offering discussion tools features by default, we will need a new task to do that. However, the necessity of that depends on input from the performance team about wether intermediary steps should be taken to reduce discussion tools usage of the parser cache.

I don't think we have an explicit ticket for removing the split, beyond it being kind of inherent in any ticket which is releasing the tool as opt-out.

I just happened upon T273072...does this ticket referring to removing the Parser Cache split?

This can be tested once T273072 is deployed.

Note: we're awaiting guidance from the Performance and Data Persistence Teams on when T273072 can be deployed. See: T280599#7323653.

@matmarex + @DLynch: can you please move this ticket to QA once T273072 is merged? [i]


i. I am assuming T273072 being merged is what will make this ticket ready for QA.

Requirements

  1. Testing: Write a patch which turns off the parser cache split; with the split disabled and all Discussion Tool features disabled, we need to make sure that everything is properly hidden.

@DLynch: would it be accurate for me to think the above is something you and Bartosz have already done and as such, it is not something @Ryasmeen needs to be concerned with testing?

Note: we'll test scenarios "3." and "6." in T291676.

@ppelberg Yes, that's the patch that was written for T273072.

@ppelberg Yes, that's the patch that was written for T273072.

Understood. Thank you, David.