Last modified: 2014-06-29 13:41:24 UTC
The letters included in the source tag used by GeSHi are too small. A user may not be able to recognize the content. Please, enlarge the tag's fontsize up to 100%.
*** This bug has been marked as a duplicate of bug 27145 ***
We generally mark newer bugs as duplicates of older bugs, not the other way around.
*** Bug 27145 has been marked as a duplicate of this bug. ***
I have to agree. Roughly since Vector came I have put a font-size:120% in my global.css for Wikimedia wikis to make them readable again. In my setup (Safari 5 / Mac) since Vector the size of code sources is now below the minimum for anti-aliasing which makes the code really pixelated and hard to read (like 8 or 10px). Noticeably smaller than normal article text.
I opened a new bug because this one was in the wrong category (and could not be changed unless closed). But to reitterate: GeSHi is inserting redundant <div style="font-family: monospace;"> and even less necessary <pre style="font-family: monospace;"> into the generated HTML. Redundant because the inline generated CSS already contains the necessary font declaration, and <pre> is already using a monospace font. The way it is currently declared also has problems in several browsers that cause the text to be too small (see bug 20706, for which a general fix has been applied, but also needs to be applied here), and also interferes with sitewide CSS (ie. common.css). What needs to be done: * Remove the inline, hardcoded font styling from the generated HTML (as it is already present in the generated CSS) * Change the default font declaration in the generated CSS to style="font-family: monospace, 'Courier New';" (see bug 20706 for explanation; the objective is not to prefer Courier New, but to trigger the correct behaviour of those borwsers)
Both these issues need to be pushed upstream to GeSHi. Can we just use !important keywords to fix this within MW? Is it even an issue which affects other GeSHi implementations?
It affects MW in that sitewide CSS may be blocked, and this may be true for other websites. I just fixed the Geshi CSS mess on en.wiki, just so I could finally assign my own font ([[en:Mediawiki talk:Common.css#Pre-formatted (and GeShi) text finally fixed properly]]). But relying on !important; remains a hack.
We can implement it in a RL'd CSS module included on GeSHi pages; I didn't mean we should make individual wikis fix it in site CSS. It only makes sense to push changes upstream if this is a problem which affects all uses of the GeSHi engine, not just its use in MediaWiki; if that's not the case, we should be doing whatever we need to at a MW level, which pretty much reduces us to !important, unless GeSHi has a hook API (I haven't checked). What CSS needs to be bundled in to straighten out the issues on MW?
The necessary CSS is already in the proper place (the generated CSS), which can hopefully be changed by means of local configuration; but it needs to have it's duplica CSS removed from the inline HTML, which can probably only be fixed upstream.
@Krinkle, The code to fix the fontsize for <source> and <syntaxhightlight> (as well as .css and .js pages) is: div.mw-geshi div pre, span.mw-geshi, pre.source-css, pre.source-javascript { font-family: monospace, "Courier New" !important; } Putting this in nl.wiki common.css (or personal CSS) will fix the problem (until GeSHi is fixed).
This may not have to be pushed upstream after all. After reading the GeSHi documentation, there are options to set inline CSS in the GeSHi headers, most notable 'set_header_content_style'. I don't know how this is implemented in the extension, but that seems to be the most logical place to fix. The current setup was probably done this way to avoid dependency on 'external' CSS (like common.css), but at the cost of having to use extranious code to fix issues.
If we want to fix it for Mediawiki, then all that is needed is to add the following code to shared.css: /* Fix so <syntaxhighlight> tags and .css and .js pages get normal text size. */ div.mw-geshi div div.mw-geshi div pre, span.mw-geshi, pre.source-css, pre.source-javascript { font-family: monospace, "Courier New" !important; }
(In reply to comment #12) > If we want to fix it for Mediawiki, then all that is needed is to add the > following code to shared.css: So? Could anyone submit the fix for this bug?
Created attachment 8597 [details] Patch for shared.css
Could someone please commit this to Wikimedia wikis?
(In reply to comment #14) > Created attachment 8597 [details] > Patch for shared.css This should not be put in shared.css, it's related to an extension which is not going to be enabled on all wikis. It might be reasonable to add it as a live-hack to the WMF cluster, though; as all WMF wikis *do* have SyntaxHighlight enabled.
What extension is that? I can't find it. If it has an associated CSS file (judging from GeShi's tendencies to throw all CSS inline, I doubt there is), it should be put in there .
There is MediaWiki:Geshi.css for CSS about the Geshi extension.
That file tends to load *after* user CSS, so that is not an option. That is one of the reasons I moved the font declaration to common.css in the first place.
The extension is called SyntaxHighlight_GeSHi. It has no CSS file, so a fix in core is out. However, if the extension can be adapted to have a CSS file, or load [[MediaWiki:Geshi.css]] ahead of site/user CSS, then that should be considered.
Comment on attachment 8597 [details] Patch for shared.css Marked patch obsolete.
*** Bug 32990 has been marked as a duplicate of this bug. ***
If I am to understand Happy-melon correctly, GeSHi is not part of core. However, if it is part of the distribution package, I do consider it to at least part of the software, and fixing this in shared.css would be appropriate. As GeSHi does not have an associated CSS file, and MediaWiki:GeSHi.css loads too late, I can't see another alternative. Otherwise, the only alternative is to recommend each project to put the following in their Common.css: /* Fix so <syntaxhighlight> tags and .css and .js pages get normal text size */ div.mw-geshi div div.mw-geshi div pre, span.mw-geshi, pre.source-css, pre.source-javascript { font-family: monospace, Courier !important; }
(In reply to comment #23) > least part of the software, and fixing this in shared.css would be appropriate. No, it's not. There is no such element by the classname "mw-geshi" outputted by core. As such styling for those elements simply don't belong in core. Adding a css file and adding it to the top loading stack in trivial, sounds like a good thing to do. However if the font-size is too small, I wonder whether: * What is setting it that low in the first place ? Are there really no CSS rules coming from Geshi ? Perhaps the font-size was hardcoded in local wiki's Geshi.css, in which case the fix is to fix that. * Perhaps it's not specific to anything "div.mw-geshi" at all, and all we need is to add a rule for "pre" in general to the Vector skin.
There already is a general rule or 'fix' for preformatted text, now annotated, that explains the root of the problem (it's not really a bug, just a side effect of the base font size). See r108142. The problem with GeSHi however is that it injects a Dynamically Generated Stylesheet in the head of every page that uses GeSHI, which re-declares the font for <pre> to plain 'monospace', negating the 'fix' in commonElements.css. Since the injected CSS loads *after* ResourceLoader has loaded the site/user CSS, the use of !important; is required in any CSS that attempts to fix GeSHI issues. It then loads Geshi.css, so putting the fixes in there is also not an option, as that would make any custamization completely impossible (as it is loaded after user CSS). Moving Geshi.css to the top loading stack would solve the problem only partly; it would still be overridden by the generated CSS, so !important is still required (as the code in common.css does now). I would also solve Geshi.css being loaded multiple times; each lang option for GeSHi generates a new CSS block in the head, including a line that loads Geshi.css. Just look at the source of [[MediaWiki talk:Common.css]] to see what I mean. A complete solution would be to move the GeSHi generated CSS to *above* the top RL modules.
Created attachment 9827 [details] Patch for extensions/SyntaxHighlight_GeSHi/geshi/geshi.php I did some digging through the GeSHi source, and here is a simpler solution; make the generated CSS perform the same 'trick' as core does, namely add a second value for font-family. This will eliminate the need for the current hack in MediaWiki:Common.css. I would still like the entire GeSHi block to be loaded before site/user CSS, so overriding no longer needs !important (but Geshi.css must still be loaded after the generated CSS, realised that a bit too late). So the order would become: 1) Generated GeSHi CSS, 2) MediaWiki:Geshi.css, 3) RL site/user CSS. And also, MW:Geshi.css should be loaded only once.
(In reply to comment #26) > Created attachment 9827 [details] > Patch for extensions/SyntaxHighlight_GeSHi/geshi/geshi.php Is that one of the GeSHi files that's linked via an SVN external? That is, is it part of GeSHi core? If so we'll need to upstream it. It looks to me like that patch ought to be completely harmless for other users, is that the case? Can you summarise why it has the effect that it does?
(In reply to comment #25) > There already is a general rule or 'fix' for preformatted text, now annotated, > that explains the root of the problem (it's not really a bug, just a side > effect of the base font size). See r108142. Wrong rev link ? (In reply to comment #25) > The problem with GeSHi however is that it injects a Dynamically Generated > Stylesheet in the head of every page that uses GeSHI, which re-declares the > font for <pre> to plain 'monospace', negating the 'fix' in commonElements.css. > Since the injected CSS loads *after* ResourceLoader has loaded the site/user > CSS, the use of !important; is required in any CSS that attempts to fix GeSHI > issues. This loading order is simply wrong. We should let the output become a module and actually make it use ResourceLoader. That'll slim the page weight, be better cacheable, and solves the order problem. (In reply to comment #25) > Moving Geshi.css to the top loading stack would solve the problem only partly; > it would still be overridden by the generated CSS, so !important is still > required (as the code in common.css does now). I didn't mean moving Geshi.css to the top. I meant, if there is no stylesheet to put this style in, we can create a _new_ module with a new stylesheet and put that in there and load it in the top. (In reply to comment #25) > I would also solve Geshi.css being loaded multiple times; each lang option for > GeSHi generates a new CSS block in the head, including a line that loads > Geshi.css. Just look at the source of [[MediaWiki talk:Common.css]] to see what > I mean. I see nothing obvious on that wiki page, can you be more specific ? (In reply to comment #25) > A complete solution would be to move the GeSHi generated CSS to *above* the top > RL modules. I'd recommend letting actually BE a module, give it position=>top and load it as any other module. Same result. (In reply to comment #26) > Created attachment 9827 [details] > Patch for extensions/SyntaxHighlight_GeSHi/geshi/geshi.php > > Is that one of the GeSHi files that's linked via an SVN external? That is, is > it part of GeSHi core? If so we'll need to upstream it. Yup.
(In reply to comment #27) > Can you summarise why it has the effect that it does? Erwin, I'm marking this patch "need-review" but please do respond to this.
Patch looks good. However we'll have to get it through upstream.
(In reply to comment #28) > Wrong rev link ? No, that is the proper one, but a link to the bug might be more prudent: bug 33496. > This loading order is simply wrong. We should let the output become a module > and actually make it use ResourceLoader. That'll slim the page weight, be > better cacheable, and solves the order problem. That entails a lot of work. The CSS is injected by geshi.php, and does so dynamically depending on what highlighting is required. The entire CSS collection for all languages GeSHi supports is over 100KB. > I didn't mean moving Geshi.css to the top. I meant, if there is no stylesheet > to put this style in, we can create a _new_ module with a new stylesheet and > put that in there and load it in the top. This would also require that GeSHi write a temporary (cachable?) CSS file which is then served by RL. (Re: multiple loading of Geshi.css) > > Just look at the source of [[MediaWiki talk:Common.css]] to see what > > I mean. > > I see nothing obvious on that wiki page, can you be more specific ? Sorry, I meant the HTML source. Each lang option creates an entire block of CSS, each with a call to mediawiki:geshi.css attached. > > Created attachment 9827 [details] > > Patch for extensions/SyntaxHighlight_GeSHi/geshi/geshi.php > > > > Is that one of the GeSHi files that's linked via an SVN external? That is, is > > it part of GeSHi core? If so we'll need to upstream it. > > Yup. I made this patch specifically for MediaWiki, since it's default skin uses a smaller-then-default font size. Other software using GeSHi might also benefit, but *may* end up with a too large font if they use the default font size. The sanest thing to do fo GeSHi is actually to remove the font-family declaration all together, since <pre> and <code> would already display in a monospace font.
(In reply to comment #31) > (In reply to comment #28) > > This loading order is simply wrong. We should let the output become a module > > and actually make it use ResourceLoader. That'll slim the page weight, be > > better cacheable, and solves the order problem. > > That entails a lot of work. The CSS is injected by geshi.php, and does so > dynamically depending on what highlighting is required. The entire CSS > collection for all languages GeSHi supports is over 100KB. I'm thinking of modules like "ext.geshi.highlight.javascript", "ext.geshi.highlight.php" etc. > > > I didn't mean moving Geshi.css to the top. I meant, if there is no stylesheet > > to put this style in, we can create a _new_ module with a new stylesheet and > > put that in there and load it in the top. > > This would also require that GeSHi write a temporary (cachable?) CSS file which > is then served by RL. CSS and JS doesn't have to be read from a static file for ResourceLoader to do it's caching and loading logic. It can be dynamically generated and concatenated from anywhere. By having them as modules they'll load faster (since they'd be minified, combined and potentially cached from one page to another and reducing raw html pay load). > (Re: multiple loading of Geshi.css) > > > Just look at the source of [[MediaWiki talk:Common.css]] to see what > > > I mean. > > > > I see nothing obvious on that wiki page, can you be more specific ? > > Sorry, I meant the HTML source. Each lang option creates an entire block of > CSS, each with a call to mediawiki:geshi.css attached. OK > > > > Created attachment 9827 [details] > > > Patch for extensions/SyntaxHighlight_GeSHi/geshi/geshi.php > > > > > > Is that one of the GeSHi files that's linked via an SVN external? That is, is > > > it part of GeSHi core? If so we'll need to upstream it. > > > > Yup. > > I made this patch specifically for MediaWiki, since it's default skin uses a > smaller-then-default font size. Other software using GeSHi might also benefit, > but *may* end up with a too large font if they use the default font size. The > sanest thing to do fo GeSHi is actually to remove the font-family declaration > all together, since <pre> and <code> would already display in a monospace font. Sounds good. So I see two tasks, the latter of which I'll assign to myself to research (not execute yet): #1: Get upstream fixed #2: Create modules for the Geshi highlighting, so they can be treated as normal citizens in RL :)
(In reply to comment #32) > #2: Create modules for the Geshi highlighting, so they can be treated as normal > citizens in RL :) Created bug 35017 for that.
I can correct the font size using <syntaxhighlight style="font-size:13px" doesn't that work?
Can you try I Gerrit change #15781 and see if it helps?
The font-size itself is not the issue. Sure one can keep incrementing it but that's a work-around. The font-size is correct. The problem is the font-family that is being overidden by Geshi with a bad font instead of Courier New. inline geshi: > font-family: monospace; .mw-code: > font-family: monospace,Courier; All we need to do is make geshi not output that inline style.
Possible fix with https://gerrit.wikimedia.org/r/48882 , this time by having our extension append a style hack to the default Geshi one.
Merged -> marking as fixed.
Please, consider reopening this bug. At least in Firefox_19 and Chrome_25, https://gerrit.wikimedia.org/r/48882 seems to me not working. As I can see, the CSS rule affected by the patch is one of those created by $css[] = $geshi->get_stylesheet( false ); but amongst those there is also, for instance, .javascript.source-javascript {font-family:monospace;} which seems to need the same fix. Adding also $geshi->set_overall_style( ' font-family: monospace, monospace;', /** preserve defaults */ true ); will complete the fix, as I can see.
To be more precise : - in Chrome, the current patch - works for <source%gt; tags - doesn't work for pages like [[w:MediaWiki:Common.js]], based on content-model - in Firefox, it doesn't work in both cases
Unassigning self, I got too many bugs right now. Will revisit later on. Firefox indeed needs a different solution :(
*** Bug 23708 has been marked as a duplicate of this bug. ***
Change 131406 had a related patch set uploaded by Bartosz Dziewoński: Make sure font size in GeSHi output is not too small https://gerrit.wikimedia.org/r/131406
(In reply to Codicorumus from comment #39) > Adding also > $geshi->set_overall_style( ' font-family: monospace, monospace;', > /** preserve defaults */ true ); > will complete the fix, as I can see. Let's try this, then. Can you test the patch I just submitted?
Change 131406 merged by jenkins-bot: Make sure font size in GeSHi output is not too small https://gerrit.wikimedia.org/r/131406
(In reply to Bartosz Dziewoński from comment #44) > (In reply to Codicorumus from comment #39) > > Adding also > > $geshi->set_overall_style( ' font-family: monospace, monospace;', > > /** preserve defaults */ true ); > > will complete the fix, as I can see. > > Let's try this, then. Can you test the patch I just submitted? Sorry for the late reply. I hoped to set a MediaWiki instance suitable for testing and then time lapsed with nothing done. At the moment I still can't do proper testing. But -- as I can see -- the fix is already online and well working. Checked with Firefox_29 and Chrome_35, both with source tags and pages based on ContentModel, also both in "MediaWiki 1.24wmf6" (it.wikipedia.org) and "MediaWiki 1.24wmf7" (www.mediawiki.org).
