Page MenuHomePhabricator

Follow-up from migration GeSHi→Pygments: Attribute and HTML issues
Closed, ResolvedPublic

Description

Below I summarize open issues (T85794#1394163) from deployment in MW1.26wmf11 in closed T85794 and further work to be done; best before wmf12.

  • style – obviously not yet implemented, important
  • class – not yet implemented
  • id – not yet implemented, used in URL, not only for decoration – All three DONE in 043969f84eb5.
  • When highlight was used in GeSHi, the entire line was marked until right margin. – DONE in 48bf989cadb9.
    • Currently only graphical characters get background.
    • Short code items are hard to find among many long lines when scrolling down.
    • They should become obvious in the rightmost area again.
  • line was used to mark (e.g. bold) every 5th line.

I am happy with the new inline parameter, which makes functionality clear, and discouraging any further usage of enclose – eh, enclose what why how?

  • inline should avoid line breaks by <code style="white-space: nowrap"
    • The code is framed, and it looks ugly to end a line with an open box and continue with an incomplete frame on next line.
    • This goes for any <code> in wikitext, btw.
    • <code> sequences are dedicated to few words, a short statement, which should always fit into one line; the current one, else next line.
    • It isn’t helpful to have a line break between else and if when such syntax is explained.

Please discuss:

  • highlight might always trigger line mode.
    • It is hard to count lines when no line numbers are provided.
    • Readers won’t complain about better orientation.

I do support T29531 which might utilize the id of the entire block, wrt to attributes and HTML issues in this task.

Details

Related Gerrit Patches:
mediawiki/extensions/SyntaxHighlight_GeSHi : masterRefactor final output formatting
mediawiki/extensions/SyntaxHighlight_GeSHi : masterHighlight background of whole lines, not just text in them

Event Timeline

PerfektesChaos raised the priority of this task from to Medium.
PerfektesChaos updated the task description. (Show Details)
PerfektesChaos added a subscriber: PerfektesChaos.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 26 2015, 9:23 AM
PerfektesChaos set Security to None.
Paladox added a subscriber: ori.Jun 27 2015, 12:04 PM

Change 221980 had a related patch set uploaded (by Bartosz Dziewoński):
Refactor final output formatting

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

Change 221990 had a related patch set uploaded (by Bartosz Dziewoński):
Highlight background of whole lines, not just text in them

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

Krinkle added a subscriber: Krinkle.Jul 1 2015, 2:56 AM

style – obviously not yet implemented, important

Important for what? What is the use case for adding additional inline styles to a syntax highlighted block?

Anomie added a subscriber: Anomie.Jul 1 2015, 5:28 PM

style – obviously not yet implemented, important

Important for what? What is the use case for adding additional inline styles to a syntax highlighted block?

I've often used it in the past to add a left margin to match the indentation on talk pages, thanks to T25674 (despite the description there, in my experience it never worked right regardless of 'encose'):

:: Foo bar?
<syntaxhighlight style="margin-left: 3.2em">
code goes here
</syntaxhighlight>
:: Ok? ~~~~

While the need for that should go away when T25674 is fixed, old pages would still be arbitrarily broken.

Since we lack per-page CSS, people might have used inline styles to change the border and/or background color of the box instead of cluttering Common.css. For example, you might use a pink background (in addition to explanatory text for a11y, of course) on "incorrect code example" boxes.

[[mw:CC]] uses it to add overflow:auto (added in this edit).

Krinkle added a comment.EditedJul 1 2015, 7:26 PM

[[mw:CC]] uses it to add overflow:auto (added in this edit).

That's exactly the kind of thing that shouldn't be done in wikitext. The kind of thing that encourages ignorance towards actually fixing the problem, thus ending up only having it fixed by power users in a small set of cases where the author noticed the problem on their own screen – and also knows how to work around it. Especially for generated content (like an extension tag) that should just work correctly in the first place.

In the specific case of adding overflow:auto, that isn't needed anymore afaik. It took 5 years, but MediaWiki now breaks lines in <pre>. Manually setting overflow:auto doesn't scale anyway, it will be forgotten in most cases and applied based on expertise of the author and whether or not the overflow bug affected the author's own device size. May as well set it everywhere.

However there are a few cases where it may function as a stop-gap for hard to solve problems. So let's keep style for now.

Change 221980 merged by jenkins-bot:
Refactor final output formatting

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

Change 221990 merged by jenkins-bot:
Highlight background of whole lines, not just text in them

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

TheDJ updated the task description. (Show Details)Jul 5 2015, 11:46 PM
TheDJ removed a project: Patch-For-Review.
matmarex updated the task description. (Show Details)Jul 6 2015, 2:55 PM
matmarex closed this task as Resolved.Jul 6 2015, 3:07 PM
matmarex claimed this task.
matmarex added a subscriber: matmarex.

Hmm, I think that we're all done when comes to "Follow-up from migration GeSHi→Pygments"!

  • Additional HTML attributes are supported now once again (but we whitelist id, class, style and dir rather than allowing everything).
  • highlight highlights the background of whole lines once again.

Line numbering no longer turning every fifth line bold is actually not a regression from Pygments migration, but a separate change made just a few days before. See T101602 and https://gerrit.wikimedia.org/r/#/c/216983/.

We should probably consider whether we want <code/> to always use white-space: nowrap in a separate task. Line-wrapping <code/> (or inline <syntaxhighlight/>) isn't a new behavior, as far as I know. Your arguments make sense, but let's think about existing articles before we jump to changing things.

I'm not sure about highlight triggering line numbering, personally I think that line numbering in code samples is almost never useful (only if you discuss the sample in detail and need to refer to lines) and makes it more difficult to copy-and-paste (which would negatively affect pages like https://www.mediawiki.org/wiki/Manual:Skinning_Part_2#Skin_styles_and_scripts_.28Resource_Loader_modules.29). Separate task, please :)