Subject: Re: [HACKERS] Small issue in online devel documentation build

From: Bruce Momjian <[email-masked]>
Date: Fri, 7 Apr 2017 21:52:01 -0400

On Fri, Mar 24, 2017 at 07:01:46AM +0100, Fabien COELHO wrote:
> 
> Hello Peter,
> 
> >I think the fix belongs into the web site CSS, so there is nothing to
> >commit into PostgreSQL here.
> 
> Indeed, the changes were only for the "remove nesting" solution.
> 
> >I will close the commit fest entry, but I have added a section to the open
> >items list so we keep track of it. (https://wiki.postgresql.org/wiki/PostgreSQL_10_Open_Items#Documentation_tool_chain)
> 
> I put forward that the quick workaround a colleague of mine suggested (aka
> something like code code { font-size: 100%; important! }) could also be
> applied to the web site CSS while waiting for a more definite answer which
> might take some pretty unknown time close to never?

Sorry I am just getting back to this.  Below I am going to cover only
the problem with the font size of nested <code> tags, and I am going to
confirm what most people already figured out.

The basic problem was already posted by Fabien, with an image example. 
The cause of the fonts being too large on Chrome is an interaction of
Chrome's default font size for different blocks, the JavaScript that is
meant to fix such mismatches, and the new nested code blocks in the PG
10 docs.

First, the JavaScript:

	https://github.com/postgres/pgweb/blob/master/media/js/monospacefix.js

There is no git history for this file except for its initial checkin in
2011, but I am pretty sure I wrote it.  What it does is to create <pre>
and <p> blocks, find the font point size, and compute a ratio.  If the
ratio is not 1, <pre>, <tt>, and <code> blocks are adjusted in size to
match <p>.  The complex part is that the JavaScript conditionally
injects CSS into the web-page to accomplish this.

The reason the PG 10 docs look fine on Linux Firefox is because the font
points sizes match so no CSS is injected.  They don't match on Chrome,
so the CSS is injected.  When the CSS hits double-embedded code blocks,
<code> <code>, it makes the font too large because it double-adjusts.

The fix, as Fabien identified, is to conditionally inject additional CSS
to be _more_ specific than the first CSS and set the font-size to a
simple '1em' so the first CSS is not called twice.  I don't think
'important!' is necessary but it would be good to test this.

Attached is a patch that can be applied to pgweb which should fix all of
this.

-- 
  Bruce Momjian  <[email-masked]>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +
diff --git a/media/js/monospacefix.js b/media/js/monospacefix.js
new file mode 100644
index 8523932..42ce913
*** a/media/js/monospacefix.js
--- b/media/js/monospacefix.js
*************** if (newMonoSize != 1)
*** 19,24 ****
  {
          document.write('<style type="text/css" media="screen">'
  					+ '#docContainer tt, #docContainer pre, #docContainer code'
! 					+ '{font-size: ' + newMonoSize.toFixed(1) + 'em;}</style>\n');
  }
  
--- 19,27 ----
  {
          document.write('<style type="text/css" media="screen">'
  					+ '#docContainer tt, #docContainer pre, #docContainer code'
! 					+ '{font-size: ' + newMonoSize.toFixed(1) + 'em;}</style>\n'
! 					/* prevent embedded code tags from changing font size */
! 					+ '#docContainer code code'
! 					+ '{font-size: 1em;}</style>\n');
  }

-- 
Sent via pgsql-www mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-www

---

From: Fabien COELHO <[email-masked]>
Date: Sat, 8 Apr 2017 19:33:15 +0900 (JST)

> 'important!' is necessary but it would be good to test this.

I did not wrote the fix, a JS-guru colleague of mine did, and he argued 
that it was useful to fix priorities between competing and contradicting 
CSS rules. Now maybe in this particular case it might work without it.

> Attached is a patch that can be applied to pgweb which should fix all of

Great, the devel doc looks ok now.

-- 
Fabien.

---

From: Bruce Momjian <[email-masked]>
Date: Sat, 8 Apr 2017 06:39:16 -0400

On Sat, Apr  8, 2017 at 07:33:15PM +0900, Fabien COELHO wrote:
> I did not wrote the fix, a JS-guru colleague of mine did, and he argued that
> it was useful to fix priorities between competing and contradicting CSS
> rules. Now maybe in this particular case it might work without it.

It would be needed for his version because he wasn't matching the exact
tags of the CSS that the JavaScript generated.

> Great, the devel doc looks ok now.

Great.

What other problems do we have with pgweb that I can work on?

-- 
Sent via pgsql-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

---

From: Robert Haas <[email-masked]>
Date: Sat, 8 Apr 2017 12:50:19 -0400

> What other problems do we have with pgweb that I can work on?

Well, the 10devel documentation doesn't believe in orange.  Compare:

https://www.postgresql.org/docs/devel/static/sql-createtable.html
https://www.postgresql.org/docs/9.6/static/sql-createtable.html

I think that needs to be fixed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Bruce Momjian <[email-masked]>
Date: Sat, 8 Apr 2017 20:32:31 -0400

> I think that needs to be fixed.

The attached CSS patch will fix that specific issue, but I am unclear if
there are more places that need color.

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +
diff --git a/media/css/text.css b/media/css/text.css
new file mode 100644
index 129735c..a910076
*** a/media/css/text.css
--- b/media/css/text.css
***************
*** 4,10 ****
  
  /* Heading Definitions */
  
! h1 {
    color: #EC5800;
  }
  
--- 4,10 ----
  
  /* Heading Definitions */
  
! h1, h2 .refentrytitle {
    color: #EC5800;
  }

---

From: Magnus Hagander <[email-masked]>
Date: Sun, 9 Apr 2017 18:59:09 +0200

On Sat, Apr 8, 2017 at 3:52 AM, Bruce Momjian <[email-masked]> wrote:

> > >items list so we keep track of it. (https://wiki.postgresql.org/
> wiki/PostgreSQL_10_Open_Items#Documentation_tool_chain)
> >

There's more history in the pgweb-old repository. You can see that this
incarnation of the file actually came from Thom Brown:

https://git.postgresql.org/gitweb/?p=pgweb-old.git;a=commitdiff;h=c67090da83079be8f268564adb63e3f0d3379f29

Your was probably the version even before that one.

> Attached is a patch that can be applied to pgweb which should fix all of
Is there any chance we can find a way to do this witha ctual CSS selectors
and not use javascript? I realize there might not be, but have we explored
the option properly on the way the site layout looks now and with
reasonably modern browsers?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

---

From: Magnus Hagander <[email-masked]>
Date: Sun, 9 Apr 2017 19:00:38 +0200

> there are more places that need color.
Does this not also affect existing versions of the docs? AFAICT it would
hit any and all <h2> tags, and we certainly have those in earlier versions?

And it also modifies a global stylesheet for *all* pages. So what about the
<h2> on all the non-docs pages?

---

From: Bruce Momjian <[email-masked]>
Date: Sun, 9 Apr 2017 22:02:28 -0400

On Sun, Apr  9, 2017 at 07:00:38PM +0200, Magnus Hagander wrote:
> Does this not also affect existing versions of the docs? AFAICT it would hit
> any and all <h2> tags, and we certainly have those in earlier versions?

It would hit any class of .refentrytitle inside of an <h2> block.  I
didn't that that happened in the old docs, but I can use an href filter
to do it only for docs of versions starting with '1' or saying 'devel'.

> <h2> on all the non-docs pages?

Again, we can restrict to only certain href patterns.

---

From: Bruce Momjian <[email-masked]>
Date: Sun, 9 Apr 2017 22:06:49 -0400

On Sun, Apr  9, 2017 at 06:59:09PM +0200, Magnus Hagander wrote:
> Is there any chance we can find a way to do this with actual CSS selectors and
> not use javascript? I realize there might not be, but have we explored the
> option properly on the way the site layout looks now and with reasonably modern
> browsers?

I realize that using JavaScript to insert CSS styles into an HTML
document is complex, but I know of no other way to scale a font style to
match another font style.  CSS doesn't give you that control.  You can
use CSS to prevent the display of certain font styles, but that hardly
seems like a win.  There is probably a way to do the entire thing only
in JavaScript, but again, that doesn't seem any better.

---

From: Magnus Hagander <[email-masked]>
Date: Mon, 10 Apr 2017 10:17:26 +0200

On Mon, Apr 10, 2017 at 4:02 AM, Bruce Momjian <[email-masked]> wrote:

> hit

Oh. That shows my love for css, i missed the fact that it will only hit
*subentries* with that class.
So it hits either "h1" *or* "h2" followed by something that's class
"refentrytitle".

Gotcha.

> to do it only for docs of versions starting with '1' or saying 'devel'.

That said, doing something like this would probably be cleaner. However, I
think doing it based on href is a bad idea -- we're bound to silently break
that somehow.

What if we changed the <div id="docContent"> tag to be something like <div
id="docContent" class="post-10">

Please suggest a better name -- but the idea being we add a class do it,
conditionally for the docs of v10 and newer (which would include devel).
That way that logic stays where it belongs, and the CSS just uses the
information to style on.

Seems reasonable?

---

From: Bruce Momjian <[email-masked]>
Date: Mon, 10 Apr 2017 09:44:16 -0400

On Mon, Apr 10, 2017 at 10:17:26AM +0200, Magnus Hagander wrote:
> What if we changed the <div id="docContent"> tag to be something like <div id=
> "docContent" class="post-10"> 
> 
> conditionally for the docs of v10 and newer (which would include devel). That
> way that logic stays where it belongs, and the CSS just uses the information to
> style on.

While we could trigger on some special tag, it is best to trigger on
something the build system does anyway so that if someone builds pre-10
docs with the new build system, they get the same output.  The attached
patch uses:

	div.refnamediv h2 .refentrytitle

and "div.refnamediv" is only generated in the new build system.

index 129735c..2714f83
*** a/media/css/text.css
--- b/media/css/text.css
***************
*** 4,10 ****

! h1 {
    color: #EC5800;
  }
  
--- 4,10 ----

! h1, div.refnamediv h2 .refentrytitle {
    color: #EC5800;
  }

---

From: Peter Eisentraut <[email-masked]>
Date: Mon, 10 Apr 2017 23:06:11 -0400

> I think that needs to be fixed.

Patch for that attached.

Also patch for improved note/warning/etc. formatting.

Another issue I saw is that tables no longer get the fancy style, e.g.,
https://www.postgresql.org/docs/devel/static/sql-syntax-lexical.html.
The style selector is table.table, which is what the build produces, but
somehow the class attribute no longer shows up on the web site.

Along with the nested <code> issue, which is already being discussed,
these are all the issues I'm aware of.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

---

From: Magnus Hagander <[email-masked]>
Date: Tue, 11 Apr 2017 14:49:44 +0200

> > I think that needs to be fixed.

At the risk of being proven wrong again, won't this affect <h2> tags in the
old documentation as well? And if so, is that something we actually want?

> Also patch for improved note/warning/etc. formatting.

That part looks safer.

> somehow the class attribute no longer shows up on the web site.

It does? The output on my laptop for that produces <table
summary="Backslash Escape Sequences" border="1"> (for example). It's
wrapped in a div with class=table. But the old code had <table border="1"
class="CALSTABLE">. And AFAICT, it's the CALSTABLE that the css is actually
matching on?

Could this be a toolchain version thing? I'm using jessie on my laptop,
which is the same one that the website is built with.

---

From: Peter Eisentraut <[email-masked]>
Date: Tue, 11 Apr 2017 10:30:36 -0400

On 4/11/17 08:49, Magnus Hagander wrote:
> At the risk of being proven wrong again, won't this affect <h2> tags in
> the old documentation as well? And if so, is that something we actually
> want?

Right.  New patch with more refined selectors attached.

> wrapped in a div with class=table. But the old code had <table
> border="1" class="CALSTABLE">. And AFAICT, it's the CALSTABLE that the
> css is actually matching on?
> 

Seems so.  Patch for that attached as well.

---

From: Magnus Hagander <[email-masked]>
Date: Wed, 12 Apr 2017 13:31:51 +0200

> Seems so.  Patch for that attached as well.
Thanks, I've applied all three patches. Your version of the color patch
also seemed slightly more appropriate than Bruce's since it only hit the
docs.css and not the global css.

Once difference I notice is that for example the "note boxes" are no longer
centered, but they do now get the new formatting. Tables now look a lot
better.

I think that only leaves the change to the javascript code that Bruce sent.
Let's see if we can figure out a way to do that one without requiring
javascript, but after that we have covered all listed issues I think?

---

From: Bruce Momjian <[email-masked]>
Date: Wed, 12 Apr 2017 09:50:03 -0400

> javascript, but after that we have covered all listed issues I think?

Well, we have been using JavaScript for years, so my patch only fixes
the JavaScript we already have.  Seems applying the fix now makes sense
and then we can ponder other methods.

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 12 Apr 2017 15:43:05 -0400

On 4/12/17 07:31, Magnus Hagander wrote:
> Once difference I notice is that for example the "note boxes" are no
> longer centered, but they do now get the new formatting.

I have committed something for that.  The issue was that the generated
HTML contained hard-coded style attributes.

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 12 Apr 2017 18:32:55 -0400

> HTML contained hard-coded style attributes.

This appears to have been successful.

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 12 Apr 2017 19:19:53 -0400

> and then we can ponder other methods.

I have googled a bit.  These two articles describe what I think is the
issue we are trying to address:

http://code.stephenmorley.org/html-and-css/fixing-browsers-broken-monospace-font-handling/

http://meyerweb.com/eric/thoughts/2010/02/12/fixed-monospace-sizing/

The recipe in the first one seems to work somewhat, but not quite as
well as the current javascript.  (The recipe in the second article is
ultimately equivalent but more verbose.)

I tend to agree with Bruce that we could just fix this within the
current framework.  But if we want to make a change, there is the
information.

---

From: Peter Eisentraut <[email-masked]>
Date: Fri, 14 Apr 2017 11:21:16 -0400

> Attached is a patch that can be applied to pgweb which should fix all of

IMO, this patch is correct and the best way to fix this.  Please proceed.

---

From: Magnus Hagander <[email-masked]>
Date: Fri, 14 Apr 2017 20:45:28 +0200

> Attached is a patch that can be applied to pgweb which should fix all of

Applied, thanks.

---

From: Peter Eisentraut <[email-masked]>
Date: Fri, 14 Apr 2017 15:36:11 -0400

>     Attached is a patch that can be applied to pgweb which should fix all of

Oops, one </style> too many.

---

From: Magnus Hagander <[email-masked]>
Date: Fri, 14 Apr 2017 21:38:32 +0200

> Oops, one </style> too many.
Indeed. Fix pushed.

---

From: Noah Misch <[email-masked]>
Date: Sat, 15 Apr 2017 00:05:31 -0400

On Fri, Apr 14, 2017 at 09:38:32PM +0200, Magnus Hagander wrote:
> On Fri, Apr 14, 2017 at 9:36 PM, Peter Eisentraut <[email-masked]> wrote:

> Indeed. Fix pushed.

I see <replaceable> formats as italic, instead of the bold-italic seen in the
9.6 docs.  (This is visible in the CREATE TABLE page, linked upthread.)  Is it
worth restoring the former formatting thereof?

---

From: Alvaro Herrera <[email-masked]>
Date: Sat, 15 Apr 2017 12:39:03 -0300

> worth restoring the former formatting thereof?

I think the bold in the original looks terrible.  Regular italic looks
much nicer to me, so I'd keep it as is.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services