Bug 535845 - [Dark Theme] Extension point description not dark
Summary: [Dark Theme] Extension point description not dark
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.8   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.9 M2   Edit
Assignee: Roland Grunberg CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks:
 
Reported: 2018-06-13 04:38 EDT by Lars Vogel CLA
Modified: 2018-08-21 03:17 EDT (History)
4 users (show)

See Also:


Attachments
Screenshot (50.07 KB, image/png)
2018-06-13 04:38 EDT, Lars Vogel CLA
no flags Details
Extension Point Description (137.35 KB, image/png)
2018-06-20 02:56 EDT, Matthias Becker CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2018-06-13 04:38:40 EDT
Created attachment 274450 [details]
Screenshot

The extension point description is not styled in the dark theme. See screenshot.
Comment 1 Lars Vogel CLA 2018-06-13 04:39:19 EDT
Roland, can you have a look? I have the vague memory that you already worked on this.
Comment 2 Roland Grunberg CLA 2018-06-14 17:00:12 EDT
This is caused by org.eclipse.jface.internal.text.html.HTMLPrinter.insertPageProlog(StringBuilder, int, URL) called from PointSelectionPage#setPointDescriptionText(String).

The description text is actually rendered in a browser (if available, mainly for things like nicely rendering bullet lists). Unforunately the insertPageProlog sets the body background colour to white on all systems.

Assuming the HTMLPrinter part isn't changing, perhaps a simple search/replace in this case is enough to fix the issue. I can push something, but it needs to be polished/improved.
Comment 3 Eclipse Genie CLA 2018-06-14 17:03:39 EDT
New Gerrit change created: https://git.eclipse.org/r/124564
Comment 4 Matthias Becker CLA 2018-06-19 02:47:26 EDT
Btw: When you click on "Open extension point description" the displayed doc is also has light background in the dark theme. The HTML is rendered in SchemaTransformer.
You could add background and foreground color rules to the CSS there to make that fit into the theme.
Comment 5 Matthias Becker CLA 2018-06-20 02:56:35 EDT
Created attachment 274544 [details]
Extension Point Description

This is a screenshot of patchset 6.
There a various problems.
1.) Hyperlink color to dark (there is a color preference for hyperlink colors. See how HTMLPrinter uses this and replaces "placeholders" in the CSS.
2.) Some text still is black
3.) The dark red test is too dark

So in general the contrast between background and text colors is too low at these places. Which makes the text hard to read. See how the JavaDoc Hover of JDT does this nicely.
Comment 6 Roland Grunberg CLA 2018-06-20 16:22:54 EDT
(In reply to Matthias Becker from comment #5)
> Created attachment 274544 [details]
> Extension Point Description
> 
> This is a screenshot of patchset 6.
> There a various problems.
> 1.) Hyperlink color to dark (there is a color preference for hyperlink
> colors. See how HTMLPrinter uses this and replaces "placeholders" in the CSS.
> 2.) Some text still is black
> 3.) The dark red test is too dark
> 
> So in general the contrast between background and text colors is too low at
> these places. Which makes the text hard to read. See how the JavaDoc Hover
> of JDT does this nicely.

The reason it looks nice in JavaDoc View is because it uses its own boilerplate stylesheet (eg. eclipse.jdt.ui/org.eclipse.jdt.ui/JavadocViewStyleSheet.css) and has HTMLPrinter.insertPageProlog(...) which sets the placeholders in that style sheet. ShowDescriptionAction relies on SchemaTransformer.

It might be possible to add some basic support for the kind of placeholder replacement (from HTMLPrinter) in SchemaTransformer but I'll also have to see how those 2 existing stylesheets (schema.css, book.css) influence an additional one.
Comment 7 Matthias Becker CLA 2018-06-21 02:44:36 EDT
(In reply to Roland Grunberg from comment #6)
> The reason it looks nice in JavaDoc View is because it uses its own
> boilerplate stylesheet (eg.
> eclipse.jdt.ui/org.eclipse.jdt.ui/JavadocViewStyleSheet.css) and has
> HTMLPrinter.insertPageProlog(...) which sets the placeholders in that style
> sheet. ShowDescriptionAction relies on SchemaTransformer.
> 
> It might be possible to add some basic support for the kind of placeholder
> replacement (from HTMLPrinter) in SchemaTransformer but I'll also have to
> see how those 2 existing stylesheets (schema.css, book.css) influence an
> additional one.
Are these 2 existing stylesheets also used in other places?
Comment 8 Lars Vogel CLA 2018-06-21 06:36:19 EDT
Should we move the schema styling to another bug? Seems like a separate topic to me.
Comment 9 Matthias Becker CLA 2018-06-21 07:09:05 EDT
(In reply to Lars Vogel from comment #8)
> Should we move the schema styling to another bug? Seems like a separate
> topic to me.

Would be okay for me.
Comment 10 Roland Grunberg CLA 2018-06-21 10:20:06 EDT
(In reply to Matthias Becker from comment #7)
> (In reply to Roland Grunberg from comment #6)
> > The reason it looks nice in JavaDoc View is because it uses its own
> > boilerplate stylesheet (eg.
> > eclipse.jdt.ui/org.eclipse.jdt.ui/JavadocViewStyleSheet.css) and has
> > HTMLPrinter.insertPageProlog(...) which sets the placeholders in that style
> > sheet. ShowDescriptionAction relies on SchemaTransformer.
> > 
> > It might be possible to add some basic support for the kind of placeholder
> > replacement (from HTMLPrinter) in SchemaTransformer but I'll also have to
> > see how those 2 existing stylesheets (schema.css, book.css) influence an
> > additional one.
> Are these 2 existing stylesheets also used in other places?

I would have thought so given that in general, the locations getting resolved originate from the product plugin (eg. org.eclipse.sdk), org.eclipse.platform, or org.eclipse.platform.doc.isv . However, grepping through the platform aggregator, it looks like book.css is only also used by the help system (org.eclipse.help.webapp), and schema.css only by PDE UI.


(In reply to Matthias Becker from comment #9)
> (In reply to Lars Vogel from comment #8)
> > Should we move the schema styling to another bug? Seems like a separate
> > topic to me.
> 
> Would be okay for me.

That would be great. It isn't entirely separate but it is a more general case. With the description text there's mainly just the plain text to render and maybe some lists to display. With the entire schema definition (eg. configuration markup, examples, etc.) the variety of content requires additional work.
Comment 11 Lars Vogel CLA 2018-06-21 10:22:20 EDT
(In reply to Roland Grunberg from comment #10)
> > Would be okay for me.
> 
> That would be great.
Can you update your patch to only cover the extension point description?
Comment 12 Lars Vogel CLA 2018-06-21 10:36:01 EDT
Matthias, could you open a new bug for the schema color issue?
Comment 13 Matthias Becker CLA 2018-06-22 02:51:58 EDT
(In reply to Lars Vogel from comment #12)
> Matthias, could you open a new bug for the schema color issue?

done
Comment 14 Lars Vogel CLA 2018-06-22 03:33:05 EDT
Thanks, Roland
Comment 16 Vikas Chandra CLA 2018-07-31 05:12:47 EDT
Can anyone (working on dark theme) verify this one?
Comment 17 Matthias Becker CLA 2018-08-01 05:22:56 EDT
verified in I20180731
Comment 18 Vikas Chandra CLA 2018-08-13 02:31:22 EDT
Roland/Mathias, can you please add a N&N entry for this one?
Comment 19 Vikas Chandra CLA 2018-08-16 10:20:11 EDT
Can someone add N&N item for this ?
Comment 20 Roland Grunberg CLA 2018-08-16 11:29:06 EDT
(In reply to Vikas Chandra from comment #19)
> Can someone add N&N item for this ?

Sorry for the delay. If no one is taking this I'll be sure to add something before the week is up.
Comment 21 Eclipse Genie CLA 2018-08-20 10:43:23 EDT
New Gerrit change created: https://git.eclipse.org/r/127717