Opened 19 months ago

Closed 11 months ago

Last modified 9 months ago

#4136 closed defect (fixed)

Adblock Plus for Chrome devtools pane uses wrong colours when the dark theme is enabled

Reported by: mapx Assignee: kzar
Priority: P3 Milestone: Adblock-Plus-1.13-for-Chrome-Opera
Module: User-Interface Keywords:
Cc: kzar, sebastian, greiner, Lain_13 Blocked By:
Blocking: Platform: Chrome
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29368730/
https://codereview.adblockplus.org/29370766/

Description (last modified by kzar)

Environment

Chrome Version 51.0.2704.106 (64-bit)
Adblock Plus 1.12.1

How to reproduce

  1. Open the Chrome developer tools.
  2. Click the "three dots" icon at the top right and then select Settings.
  3. On the Settings -> Preferences page select "Theme: Dark" under the "Appearance" header.
  4. Close the Settings window and select the "Adblock Plus" pane in the developer tools.
  5. Re-load the inspected page so some blocked/allowed requests are listed.

Observed behaviour

The colours used for the Adblock Plus devtools pane are wrong. The text is often black on black, and sometimes the background colours do not match the dark theme. (See the uploaded image.)

Expected behaviour

The colours used in the Adblock Plus devtools pane should adapt to the light/dark theme selection.

Notes

The inspector itself can be inspected using the Ctrl-Shift-I (Ctrl-Apple-I for Macs) hotkey as long as the inspector is undocked from the page.

What to change

Use chrome.devtools.panels.themeName (see Chromium Issue 608869) to apply additional styles in case of themes being applied. Note that while we can make specific optimizations to the "dark" theme we may need to have some generic fallback style for any other ones that may be added to avoid future breakage.

Update the adblockplusui dependency to hg:575d910d9d2e git:612cb73, which will also include an unrelated change to remove some legacy code and an unrelated change which adds antiadblockinit.js from adblockpluscore.

Attachments (2)

dark.PNG (40.1 KB) - added by mapx 19 months ago.
adblockplus-devtools-dark-theme.png (70.2 KB) - added by kzar 12 months ago.

Download all attachments as: .zip

Change History (20)

Changed 19 months ago by mapx

comment:1 Changed 19 months ago by mapx

  • Description modified (diff)

comment:2 Changed 17 months ago by kzar

  • Description modified (diff)
  • Priority changed from Unknown to P4
  • Ready set
  • Summary changed from ABP panel incorrectly styled in chrome dev tools - dark theme to Adblock Plus for Chrome devtools pane uses wrong colours when the dark theme is enabled

comment:3 Changed 17 months ago by kzar

  • Component changed from Platform to User-Interface
  • Priority changed from P4 to Unknown

(I just realised the files causing this problem live in the adblockplusui repository, so I'm moving this issue to User-Interface and leaving the prioritisation up to Thomas.)

comment:4 Changed 17 months ago by kzar

  • Description modified (diff)

comment:5 Changed 17 months ago by greiner

  • Description modified (diff)
  • Keywords externaldependency added
  • Priority changed from Unknown to P3

According to Chromium Issue 608869 they added chrome.devtools.panels.themeName a few weeks ago so we should be able to use it as soon as it lands in Chrome's dev channel.

Note that we can't use the body's -theme-with-dark-background class because our panel is running inside of an iFrame.

At the moment the only thing we could do is set html {background-color: #FFF;} to ensure that our content remains usable.

comment:6 Changed 12 months ago by mapx

  • Cc Lain_13 added

comment:8 Changed 12 months ago by kzar

  • Keywords externaldependency removed
  • Owner set to kzar

comment:9 Changed 12 months ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

Changed 12 months ago by kzar

comment:10 Changed 12 months ago by abpbot

A commit referencing this issue has landed:
Issue 4136 - Support Chrome's dark devtools theme

comment:11 Changed 12 months ago by kzar

  • Description modified (diff)

(Note the dependency update is not blocked by #4715 since antiadblockinit.js is added to adblockplusui instead of removed.)

comment:12 Changed 12 months ago by kzar

  • Review URL(s) modified (diff)

comment:13 Changed 11 months ago by sebastian

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next

comment:14 Changed 11 months ago by abpbot

A commit referencing this issue has landed:
Issue 4136 - Update adblockplusui dependency

comment:15 Changed 11 months ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:16 follow-up: Changed 11 months ago by Ross

In older Chrome's, like 49, the ABP panel now has a white background and is usable again. In more recent Chrome's, like 54, the ABP now follows the dark theme.

I have a question though: Where are the red/greens in the ABP panel list from? (Such as when items are blocked or exceptions). Are they Chrome constants or colours we have chosen? Because they are really unreadable/unusable to someone with moderate colour blindness (can see colours but often mistake/blur them). Would anyone agree with that?

If they are our colours, we should fix it, otherwise I'll find/create a Chrome issue.

comment:17 in reply to: ↑ 16 Changed 11 months ago by kzar

Replying to Ross:

I have a question though: Where are the red/greens in the ABP panel list from? (Such as when items are blocked or exceptions). Are they Chrome constants or colours we have chosen? Because they are really unreadable/unusable to someone with moderate colour blindness (can see colours but often mistake/blur them). Would anyone agree with that?

They are our colours. I tried my best to pick something appropriate but we can update them if you / someone has an idea for how we could improve them.

comment:18 Changed 9 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

This is fine then. I will open a ticket about shades.

ABP 1.12.4.1739
Chrome 49 / 56 / Windows 10
Chrome 56 / OS X 10.12
Chrome 56 / Ubuntu 16.04
Opera 37 / 41 / Windows 7
Safari 10 / OS X 10.12

Note: See TracTickets for help on using tickets.