MF uses 22px left padding to hold space for the expand/collapse icon, however this doesn't behave like the real icon when more content is added to the heading.
| Before JS | After JS |
| Esanders | |
| Dec 6 2024, 12:09 PM |
| F57784149: image.png | |
| Dec 6 2024, 12:09 PM |
| F57784151: image.png | |
| Dec 6 2024, 12:09 PM |
MF uses 22px left padding to hold space for the expand/collapse icon, however this doesn't behave like the real icon when more content is added to the heading.
| Before JS | After JS |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | Release | None | T84936 Release VisualEditor-MediaWiki as "1.0" | ||
| Open | None | T50429 [Epic] Support editing parts of a page in VisualEditor-MediaWiki | |||
| Open | None | T54365 Explore performance gains from progressive (JIT?) de-alienation in VisualEditor | |||
| Open | None | T396539 Introduce a way to edit a span of text from within read mode | |||
| Open | None | T385558 Improve copy and paste from read mode to VE | |||
| Open | None | T174303 Copy-pasting linked ISBN numbers from view mode HTML into VisualEditor inserts wikitext links to Special:BookSources (it should turn them into magic links?) | |||
| Open | Feature | None | T54091 The read HTML should have hinting to allow full DOM copying (as opposed to just rich copying) from read mode into VE surfaces | ||
| Open | None | T55784 [EPIC] Use Parsoid HTML for all page views | |||
| Open | None | T272987 [EPIC] Parsoid read views for DiscussionTools | |||
| In Progress | None | T381655 FOUC when loading talk pages on mobile using Parsoid read views |
Change #1101034 had a related patch set uploaded (by Esanders; author: Esanders):
[mediawiki/extensions/MobileFrontend@master] Reserve space for collapsible heading icon more accurately
This will become a priority once Parsoid read views for talk pages start happening but it currently doesn't affect any production sites.
There is an open patch from Ed here that touches article page views. Given issues we've had with parsoid and non-Parsoid views, web team should be involved with code review for this change.
Change #1113229 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/extensions/DiscussionTools@master] Fix Parsoid flash of unstyled content on talk page load
I don't think it's a good idea to use downstream hacks. The upstream fix I proposed creates a more accurate placeholder for the expand/collapse button in the same place that the expand collapse button is defined.
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/1101034/comments/e5cf1cb8_a9c421aa
I think generally we'd like to avoid changing these styles wherever possible given Wikivoyage relies on these for page views
Is there any indication that Wikivoyage have customised these styles in any way that is likely to cause a regression? The possibility of a minor visual regression on mobile Wikivoyage does not seem like the best reason to avoid doing the right thing and making our code more robust and maintainable.
resources/mobile.init.styles/main.less runs on all wikis not just Wikivoyage so a change here is risky while we are focused on other parts of the code. Both solutions seem hacky TBH since they are changing the default styles of a skin which is neither the responsibility of MobileFrontend or DiscussionTools. I recommended the latter given it removes the web team from blocking you from progress.
The current situation means any time the web team or editing team modify the headings we risk breaking each other's experiences and set ourselves up for a game of whack a mole. If we want our code more robust and maintainable then web and editing team should dedicate some time creating stable APIs in PHP and JS for appending items to section headings with well understood contracts. The community has already requested one for gadgets [1] so I think that would a be a good use of our time. What do you think?
[1] T337286: Gadget API for adding buttons to section titles
MobileFrontend implements collapsible headings, so it is responsible for the styling of them, not the skin (yet), both before and after JS loads. This is the status quo. All my patch does is fix the before-JS implementation from a naive padding-left: 22px to one which more closely matches the behaviour of after-JS experience. Implementing it downstream in DT would indeed be a hack, which is why I want to do this in MF.
A quick code search and on-wiki search for mf-collapsible-heading gave no results and I'm unaware of anyone relying on this styling specificially (although you mentioned Wikivoyage which is I asked about it).
Better APIs would be nice, but for now I'm just trying to fix the current behaviour.
Better APIs would be nice, but for now I'm just trying to fix the current behaviour.
I understand that you’re eager to resolve this quickly, but the proposed patch affects all pages in order to address an issue specifically related to talk pages.
The web team is currently managing multiple versions of mobile markup (Parsoid mobile, legacy parser mobile, Parsoid with $wgParserEnableLegacyHeadingDOM enabled, and legacy parser with $wgParserEnableLegacyHeadingDOM disabled). Given current priorities, this is not our primary focus. Any change in this area, no matter how small, requires a context switch and carries a risk of additional workload for the team.
To help move this forward, I’m suggesting an alternative approach that limits the fix to talk pages without involving the web team. Additionally, I’m open to collaborating on a long-term solution, which seems like a more efficient use of our time than repeatedly addressing isolated breakages.
The team (with me as the one who moved it: T381655#10459386) has already indicated that we won’t be able to take this on during the current quarter as proposed. However, if you’d like to revisit the prioritization, I encourage bringing it up in the next web/editing sync, as my understanding is that these meetings are intended for discussions around shifting priorities.
Not quickly, but correctly. Patching DiscussionTools is just building up technical debt, whereas our proposed patch provides a better placeholder that should work for both talk pages and any future use cases.
Perhaps we can take responsibility for QA / fixing regressions / rolling back if it proves to more complex than it appears?
Change #1132030 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] WIP: Server side render the collapse arrow in MobileFrontend
Perhaps we can take responsibility for QA / fixing regressions / rolling back if it proves to more complex than it appears?
My concern here is that we probably won't know about issues until a few months later when Parsoid rolls out further to more wikis. The problem we have right now is not many people are reporting Parsoid bugs as they happen and they keep leaking into the HTML here. My understanding is that Parsoid's goal is to be everywhere by end of next quarter and it's likely the web team will have a hypothesis relating to this in Q4 so would want to be closely involved with any heading changes. Having less variations of this HTML will be helpful. I've also recommended we drop support for legacy markup to make this easier in our world (
Not quickly, but correctly. Patching DiscussionTools is just building up technical debt
I get it. Patching MobileFrontend is also building up technical debt for web team, and that's what I'm trying to get across here. We need to pay off this technical debt as part of the Parsoid release.
Since we are both complaining about technical debt.. it seems like either way one of our team's here ends up with code they'd consider technical debt.. I really think it would be worthwhile relying less on hacks and formalizing this more.
I think ideally, the ext-discussiontools-init-section-metadata and h element would be ancestors of the same element so we could rely on HTML for layout. Are there reasons you didn't mark it up that way?
Given DiscussionTools has shown the concept of adding context to headings is useful (also the gadget community is regularly asking for this). Is this something the editing team would be interested in collaborating on formalizing in PHP/JS (so we're not resorting to badly-defined CSS hacks that need to be kept in sync with Codex)?
Currently:
<div class="mw-heading">
<span class="mf-icon mf-icon-expand mf-icon--small indicator "> </span>
<h2></h2>
<span class="mw-editsection" />
<span class="ext-discussiontools-init-section-overflowMenuButton" />
<div class="ext-discussiontools-init-section-bar"/>
</div>Ideal proposed version:
<div class="mw-heading">
<div class="mw-heading-start">
</div/>
<span class="mf-icon mf-icon-expand mf-icon--small indicator "> </span>
<div class="mw-heading-main">
<h2></h2>
<div class="ext-discussiontools-init-section-bar"/>
</div>
<div class="mw-heading-end">
<span class="mw-editsection" />
<span class="ext-discussiontools-init-section-overflowMenuButton" />
</div>
</div>I'm not sure whether it makes sense to do this first in DiscussionTools, MobileFrontend or in the parser, but I'm open to either option.
In lieu of that, really the right thing to do here is the above - to make the parsoid version more consistent with legacy (which is the above patch) rather than fiddling more with this CSS technical debt in MobileFrontend.
Change #1113229 abandoned by Jdlrobson:
[mediawiki/extensions/DiscussionTools@master] Fix Parsoid flash of unstyled content on talk page load
Reason:
Ed considers this tech debt, so let's not do this. Suggested https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/1132030/ instead.
Change #1132030 abandoned by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] WIP: Server side render the collapse arrow in MobileFrontend
Reason:
Abandoning while I'm out. Happy for someone to fork or restore while I'm gone.
I get it. Patching MobileFrontend is also building up technical debt for web team, and that's what I'm trying to get across here. We need to pay off this technical debt as part of the Parsoid release.
Since we are both complaining about technical debt.. it seems like either way one of our team's here ends up with code they'd consider technical debt.. I really think it would be worthwhile relying less on hacks and formalizing this more.
This isn't adding any tech debt to MF - it's just improving the existing approach (specifically - using CSS to create a placeholder for the expand/collapse buttons). While this currently only manifests when DT is enabled, it is still a MF bug.
Given DiscussionTools has shown the concept of adding context to headings is useful (also the gadget community is regularly asking for this). Is this something the editing team would be interested in collaborating on formalizing in PHP/JS (so we're not resorting to badly-defined CSS hacks that need to be kept in sync with Codex)?
I'd be happy to work on that in a separate task along with CTT, but I don't want it to block this fix, as this issue is already affecting wikis where Parsoid is deployed.
Change #1101034 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Reserve space for collapsible heading icon more accurately
e.g. on https://en.m.wikipedia.beta.wmcloud.org/wiki/Talk:Main_Page?useparsoid=1 -- the FOUC is only really visible at narrow phone window sizes.
Possibly reported by a user as a dark-mode bug at https://www.mediawiki.org/wiki/Parsoid/Feedback#c-Cscott-20251122034700-Coolguyyo-20251116035900 ?