Bug 515972

Summary:

Javadoc hover strip background should be the same as Javadoc body's background

Product:

[Eclipse Project] Platform

Reporter:

Leo Ufimtsev <lufimtse>

Component:

Assignee:

Leo Ufimtsev <lufimtse>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

daniel_megert, markus.kell.r

Version:

4.7

Target Milestone:

4.7 M7

Hardware:

OS:

Linux

See Also:

https://git.eclipse.org/r/96065
https://git.eclipse.org/r/96066
https://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=73ff2c16d11d91e0ca6c479cf07cb665309d3bbd
https://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=1ced1b594fcdd3fd43ef1529449ef77eb8cadaa4
https://bugs.eclipse.org/bugs/show_bug.cgi?id=516003
https://bugs.eclipse.org/bugs/show_bug.cgi?id=516420

Whiteboard:

Bug Depends on:

Bug Blocks:

507072

Attachments:

Description	Flags
Problem Screenshot - different colors	none
Solution screenshot - Same colors	none

Description Leo Ufimtsev

2017-04-28 15:52:18 EDT

This is a consolidation of Bug 508776 and Bug 508494, for clarity and consistency.

Currently, the strip at the bottom of Javadoc hover has a different background from the main body. (See "different color" screenshot).

It should look the same. (See "Fix, same color" screenshot).

The solution consists of two parts:

1) Status bar should get painted.
git:eclipse.platform.text project:jface.text->AbstractInformationControl.java
This Control doesn't set colors for it's status bar element upon initialization and set(Foreground|Background) methods.

2) JavaDoc hover should set and update AbstractInformationControl's colors.
git:eclipse.jdt.ui project:eclipse.jdt.ui -> JavadocHover.java
a) When HoverControlCreator creates a BrowserInformationControl (which is an extension of AbstractInformationControl), it should set foreground/background colors as per Javadoc user preference
b) It should also update them when user changes "Javadoc" color in preferences.

DARK THEME NOTE:
At time of writing, I couldn't get the mechanism to work on Gtk's Dark theme. The dark-theme overrides the colours of status composite manually independent of JDT.
I.e 3 things happen:
1) AbstractInformationControl is created with default colors
2) JavadocHover applies foreground/background colors
3) Dark theme overrides composite/label colors with it's own styles. (<< problem).
As a result the strip at the bottom in GTK's dark theme receives theaming colours. I will investigate this separately and will probably need some help from someone knowledgeable.
For now, I'll submit patch 1 & 2 to fix on regular Eclipse.

Comment 1 Leo Ufimtsev

2017-04-28 15:52:51 EDT

Created attachment 268076 [details]
Problem Screenshot - different colors

Comment 2 Leo Ufimtsev

2017-04-28 15:53:12 EDT

Created attachment 268077 [details]
Solution screenshot - Same colors

Comment 3 Eclipse Genie

2017-04-28 16:21:21 EDT

New Gerrit change created: https://git.eclipse.org/r/96066

Comment 4 Leo Ufimtsev

2017-04-28 16:25:12 EDT

*** Bug 508494 has been marked as a duplicate of this bug. ***

Comment 5 Leo Ufimtsev

2017-04-28 16:29:15 EDT

*** Bug 508776 has been marked as a duplicate of this bug. ***

Comment 6 Leo Ufimtsev

2017-04-28 16:31:56 EDT

Awaiting patch reviews.

Comment 7 Eclipse Genie

2017-05-01 11:24:49 EDT

Gerrit change https://git.eclipse.org/r/96065 was merged to [master].
Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=73ff2c16d11d91e0ca6c479cf07cb665309d3bbd

Comment 8 Eclipse Genie

2017-05-01 12:32:10 EDT

Gerrit change https://git.eclipse.org/r/96066 was merged to [master].
Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=1ced1b594fcdd3fd43ef1529449ef77eb8cadaa4

Comment 9 Leo Ufimtsev

2017-05-03 11:29:08 EDT

NOTE TO SELF:
Below are notes for dark-theme related work:
(To be used to investigate strip on the bottom on dark theme). 
Although at the moment it looks pretty decent on Dark theme, it is a slightly lower priority for now.

/ ---- Apply skin for dark theme. But not optimal performance?
JavadocHover.java:doCreateInformationControl() ...
// TODO - remove this listener if disposed?
//                                Display.getDefault().addListener(SWT.Skin, new Listener() {
////                                iControl.fStatusLabel.addListener(SWT.Skin, new Listener() {
//                                        @Override
//                                        public void handleEvent(Event event) {
//                                                ColorRegistry registry2 = JFaceResources.getColorRegistry();
//                                                Color fgRGB2 = registry2.get("org.eclipse.jdt.ui.Javadoc.foregroundColor"); //$NON-NLS-1$ 
//                                                Color bgRGB2 = registry2.get("org.eclipse.jdt.ui.Javadoc.backgroundColor"); //$NON-NLS-1$ 
//                                                iControl.setStatusColors(fgRGB2, bgRGB2);
//                                        }
//                                });


// ---- Find what's triggering color change by dark theme.
Control:
public void setForeground (Color color) {
//        if (this instanceof Label) {
//                Label label = (Label) this;
//                if (label.getText().contains("F2")) {
//                        String col = "";
//                        if (color != null)
//                                col = color.toString();
//                        System.out.println("LABEL COLOR CHANGE:" + col);
//                        if (col.contains("238")) {
//                                System.out.println("Gray");
//                        }
//                }
//        }

// Don't style that label
org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyles(Object, boolean, boolean)
// if ("Label".equals(elt.getNodeName())) {
// if (elt instanceof Widget) {
// Widget widget = (Widget) element;
// Label label = (Label) widget;
// if (label.getText().contains("F2")) {
// System.out.println("LABEL COLOR CHANGE AT CSS LEVEL");
// return;
// }
// }
// }