Abstract Text Editor: don't change StyledText by CSS.

[DenisUngemach]

Fix for #869

Change-Id: Ib739afdc301c4e37dcbbe37ef063ed32cb7f53d7

[DenisUngemach]

In this change i just deactivate the CSS stylesheet, which overrides the colors from the "Text Editors" preferences.
Can someone confirm whether this is the right way to solve #869 .

And by the way issue #869 also affects the java class editor. That can be checked with the background color.

[BeckerWdf]

Fix for #869

Change-Id: Ib739afdc301c4e37dcbbe37ef063ed32cb7f53d7

We don't use Gerrit. So we don't need a Change-Id

[github-actions]

Test Results

     900 files  ±0       900 suites  ±0   1h 9m 16s ⏱️ + 25m 10s
  7 468 tests ±0    7 296 ✔️ ±0  153 💤 ±0  15 ❌ ±0  4 🔥 ±0 
23 553 runs  ±0  23 025 ✔️ ±0  509 💤 ±0  15 ❌ ±0  4 🔥 ±0 

For more details on these failures and errors, see this check.

Results for commit c7a090b. ± Comparison against base commit 6854743.

♻️ This comment has been updated with latest results.

[DenisUngemach]

This feature has already been used in:
See EGit's UIUtils class

[vogella]

@BeckerWdf what is the status here with the review? Did you test the change?

[BeckerWdf]

@DenisUngemach will update his PR. I then will do a manual test.

[sratz]

In princple, this works well.

But we were wondering whether disabling CSS completely is the way to go. The preferences only deal with foreground + background colors, a CSS might do more than just that.

In theory that is - in practice, the CSS found in Platform only deals with foreground + background colors as well.

So we are probably safe here and if there are no objections we can start with this and wait for feedback.

If needed, we could maybe later introduce a more fine-granular E4 property, such as:

setData("org.eclipse.e4.ui.css.disabledProperties", "background-color,foreground-color")

However, your alternative PR #1349 also has an additional condition getPreferenceStore() != null
https://github.com/eclipse-platform/eclipse.platform.ui/pull/1349/files#diff-c37eb3db119ede5b1695bb0217864648bfa40dfaafcfa25c7bbd7131d69ed641R3401

That might be useful to add as a safeguard in case AbstractTextEditor is used in a non-platform context without attached preference (is that even possible?). In that case, we wouldn't want to disable CSS.

[vogella]

I don't think the extra check is necessary.

a CSS might do more than just that.

As for the colors of the syntax highligthing, we manipulate the preference values directly, so we should be ok.

[BeckerWdf]

I did a manual test on macOS.
Without this change I can reproduce this issue
With this change the issue is gone. Change to background and foreground color in the TextEditors preference page are not correctly handled. I teste editor close and re-opening as well as workbench restart.
All looks good for me.

[vogella]

GH says, you still require changes @BeckerWdf ?

[BeckerWdf]

GH says, you still require changes @BeckerWdf ?

Let's wait for the build..

[vogella]

GH says, you still require changes @BeckerWdf ?

Let's wait for the build..

We (should) always wait for the build. I wanted to know if you still require changes in this PR? @DenisUngemach could finish it in this case.

[BeckerWdf]

Are version number increases necessary for this change?

[vogella]

Are version number increases necessary for this change?

No, the build would fails if version increases are necesarry. I did not check if the test errors relate to this change.

[BeckerWdf]

No, the build would fails if version increases are necessary.

So has the version number already been increased?

I did not check if the test errors relate to this change.

The seem to be unrelated to me.

[BeckerWdf]

Test failures are not related and already reported in #1351

[vogella]

Cool, thanks @DenisUngemach and @BeckerWdf

[DenisUngemach]

Cool, thanks @DenisUngemach and @BeckerWdf

You're welcome. Actually my first idea was to create a custom css configuration: "CSS support for custom widgets"for the AbstractTextEditor and set there the color configuration. But this didn't work that well because somewhere in the compatibility handler css class and css id will be defined:

And i only wanted to write the css class. Is this css ID writing justified? Maybe these base layers should override css class, but i think the css ID is over the top.

Best regards,
Denis