Page MenuHomePhabricator

mw-collapsible-toggle doesn't show custom toggle for li elements
Closed, ResolvedPublic

Description

The fix of T137676 broke the custom toggle display for li elements. Compare the following code:

<p>
    <div class="mw-collapsible mw-collapsed">
        <div class="mw-collapsible-toggle" style="float: none;">This custom toggle displays correctly.</div>
        <div class="mw-collapsible-content">This text is collapsed.</div>
    </div>
</p>
<ol>
<li>
    <div class="mw-collapsible mw-collapsed">
        <div class="mw-collapsible-toggle" style="float: none;">This shows a default toggle rather than the custom toggle.</div>
        <div class="mw-collapsible-content">This text is collapsed.</div>
    </div>
</li>
</ol>

Please correct this error or remove the T137676 fix.

Thanks!

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 20 2016, 3:21 PM
matmarex updated the task description. (Show Details)Aug 20 2016, 6:56 PM

@Dave_Braunschweig Is this affecting some prominent pages on Wikimedia wikis or something? I'm wondering what kind of priority this should have (i.e., should I work on it on weekend? :P).

I have submitted a patch fixing this bug: https://gerrit.wikimedia.org/r/#/c/305807/

But I submitted it to a wrong changeset. Now I don't know what to do.

Please help me to move changes from https://gerrit.wikimedia.org/r/#/c/305807/ to https://gerrit.wikimedia.org/r/#/c/296163/

@matmarex

It's been broken since June. Not necessary to work on it over the weekend unless you want to. It is somewhat functional in its current state but not desirable.

It is used on hundreds of wikiversity pages and would be good to get the fix in this week, as the fall semester is just starting at most US colleges.

Thanks!

I have submitted a patch fixing this bug: https://gerrit.wikimedia.org/r/#/c/305807/
But I submitted it to a wrong changeset.

@VictorPorton: Why would you think so? :) 296163 was merged two months ago. Hence it is complete and closed.
Separate/isolated new code change means separate new changeset.
Following https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines would be welcome though, to link to this task.

Change 305807 had a related patch set uploaded (by VictorPorton):
jquery.makeCollapsible: Support for .mw-collapsible-toggle inside <li>

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

matmarex closed this task as Resolved.Aug 22 2016, 3:23 PM
matmarex assigned this task to VictorPorton.
matmarex removed a project: Patch-For-Review.

@VictorPorton As far as I can tell you did everything exactly right, I only added Bug: T143484 to the commit message to associate the commit with this task. The patch looks reasonable and fixes this issue, thanks!

@Dave_Braunschweig The fix has been merged and will be deployed to Wikimedia wikis this week with MediaWiki version 1.28.0-wmf.16, per the release schedule.

Change 305807 merged by jenkins-bot:
jquery.makeCollapsible: Support for .mw-collapsible-toggle inside <li>

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

The fix of T137676 broke the custom toggle display for li elements.

Could this have unit tests, considering it affects layout and dom structure?

Let's add a unit test this time?