1293099 - Extend popup background color to arrow

Status: VERIFIED FIXED

(WebExtensions :: Frontend, enhancement)

Description

[Kris Maglione [:kmag]]

+++ This bug was initially created as a clone of Bug #1280128 +++

The minor styling fixes in bug 1280128 will hopefully be uplifted to 49 or 50. These changes should land in 51, after they've spent a week or two baking in nightly, and ride the trains as normal.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1293099: Fill panel arrow and arrow content with browser body's background color. ui-r=maritz

Review commit: https://reviewboard.mozilla.org/r/69870/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69870/

Comment 2

[Kris Maglione [:kmag]]

Comment on attachment 8778708 [details]
Bug 1293099: Fill panel arrow and arrow content with browser body's background color.  ui-r=maritz

Screenshots:

OS-X: https://people.mozilla.org/~kmaglione/images/3d6666a107b62931.png
Linux: https://people.mozilla.org/~kmaglione/images/0024d8e82bc8e190.png

Comment 3

[Markus Jaritz [:designakt] (UX)]

Looks great. How does it look in Windows?

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8778708 [details]
Bug 1293099: Fill panel arrow and arrow content with browser body's background color.  ui-r=maritz

https://reviewboard.mozilla.org/r/69870/#review67166

::: browser/components/extensions/ext-utils.js:327
(Diff revision 1)
> +        let bgColor = colorUtils.colorToRGBA(background);
> +        if (bgColor.a == 1) {
> +          panelBackground = background;
> +          let borderColor = this.borderColor || background;
> +
> +          panelArrow = `url("data:image/svg+xml,${encodeURIComponent(`<?xml version="1.0" encoding="UTF-8"?>

It would be nice if we could use the code from bug 1058040 to save this as a regular SVG file, but I think we probably want to uplift this to versions of Firefox that don't have that implemented yet…

All in all, looks good to me.  Thanks for getting this working, Kris!

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8778708 [details]
Bug 1293099: Fill panel arrow and arrow content with browser body's background color.  ui-r=maritz

https://reviewboard.mozilla.org/r/69870/#review67166

> It would be nice if we could use the code from bug 1058040 to save this as a regular SVG file, but I think we probably want to uplift this to versions of Firefox that don't have that implemented yet…

Hm. Interesting. I did try to find some way of doing that, but I gave up. And I'm not sure this will help much either, since it seems like we'd need to actually embed the image in an <svg> element. Which might be doable, but is a far bigger task.

Comment 6

[Kris Maglione [:kmag]]

(In reply to Markus Jaritz [:maritz] (UX) from comment #3)
> Looks great. How does it look in Windows?

Here's Windows 7:

https://people.mozilla.com/~kmaglione/images/7b6ab40b9d39f22f.png

I don't have access to Windows 8 or 10 at the moment.

Comment 7

[Kris Maglione [:kmag]]

Comment on attachment 8778708 [details]
Bug 1293099: Fill panel arrow and arrow content with browser body's background color.  ui-r=maritz

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69870/diff/1-2/

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8778708 [details]
Bug 1293099: Fill panel arrow and arrow content with browser body's background color.  ui-r=maritz

Re-requesting review since I had to change the way I was handling the CSS variables after I rebased.

Comment 9

[Markus Jaritz [:designakt] (UX)]

Comment on attachment 8778708 [details]
Bug 1293099: Fill panel arrow and arrow content with browser body's background color.  ui-r=maritz

Looks great. Thanks. How does it look when opening the panel? I mean, does the Panel start white, and change to the body color while visible? Or does that happen before it becomes visible?

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8778708 [details]
Bug 1293099: Fill panel arrow and arrow content with browser body's background color.  ui-r=maritz

https://reviewboard.mozilla.org/r/69870/#review67622

This also still looks good.  :)

Comment 11

[Kris Maglione [:kmag]]

(In reply to Markus Jaritz [:maritz] (UX) from comment #9)
> Looks great. Thanks. How does it look when opening the panel? I mean, does
> the Panel start white, and change to the body color while visible? Or does
> that happen before it becomes visible?

It depends. For pageAction popups, it's always the correct color from the start. For browserAction, there's sometimes a noticeable flash where the arrow is the original color, and the popup has zero height/zero-width-beyond-the-width-of-the-arrow. There's a separate bug for that, and I'm still trying to find a good solution.

Comment 12

[Markus Jaritz [:designakt] (UX)]

(In reply to Kris Maglione [:kmag] from comment #11)
> For browserAction, there's sometimes a noticeable flash where the
> arrow is the original color, and the popup has zero
> height/zero-width-beyond-the-width-of-the-arrow. There's a separate bug for
> that, and I'm still trying to find a good solution.

Can you please cc me on that bug. Or give me the bug-number. Thx.

Comment 13

[Kris Maglione [:kmag]]

(In reply to Markus Jaritz [:maritz] (UX) from comment #12)
> (In reply to Kris Maglione [:kmag] from comment #11)
> > For browserAction, there's sometimes a noticeable flash where the
> > arrow is the original color, and the popup has zero
> > height/zero-width-beyond-the-width-of-the-arrow. There's a separate bug for
> > that, and I'm still trying to find a good solution.
> 
> Can you please cc me on that bug. Or give me the bug-number. Thx.

Yes, it's bug 1259093.

Comment 14

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e472c47e7912625a1a9cadf6dcfb7bcec2afc686
Bug 1293099: Fill panel arrow and arrow content with browser body's background color. r=bwinton ui-r=maritz

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e472c47e7912

Comment 16

[Vasilica Tamas, QA [:vtamas]]

Attachment: black_menu_for_googletm-11.2.1-fx.xpi

I was able to reproduce the initial issue on  firefox 50.0a1 (2016-06-14) under Windows 10 64-bit.

Verified fixed on  Firefox 52.0a1 (2016-10-05) and  Firefox 51.0a2 (2016-10-05) under Windows 10 64-bit, Windows 7 64-bit, Windows 8.1 64-bit, Ubuntu 12.04 64-bit and Mac OS X 10.12.1.  The arrow is successfully filled using the background color while testing with the webextension mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1259093#c28 , but I’ve noticed that the issue still reproduces for the attached webextension (http://screencast.com/t/AHXsNOSqq).

Kris, what's your opinion? Is the black_menu webextension issue a different bug and should be treated separately?

Comment 17

[Kris Maglione [:kmag]]

That extension seems to have a couple of issues of its own:

1) It sets a border radius for the <body> element, which leads to clipped corners on platforms where we don't use rounded popups.
2) It uses a white background for its <body> element, which is what we use for the arrow.

So, while those issues should be fixed, they would need to be fixed in the add-on itself.

Comment 18

[Vasilica Tamas, QA [:vtamas]]

(In reply to Kris Maglione [:kmag] from comment #17)
> That extension seems to have a couple of issues of its own:
> 
> 1) It sets a border radius for the <body> element, which leads to clipped
> corners on platforms where we don't use rounded popups.
> 2) It uses a white background for its <body> element, which is what we use
> for the arrow.
> 
> So, while those issues should be fixed, they would need to be fixed in the
> add-on itself.

I understand, so based on Comment 17 and Comment 16 I am marking this bug as Verified.

Comment 20

[Leif-AMO]

(In reply to Kris Maglione [:kmag] from comment #17)
> That extension seems to have a couple of issues of its own:
> 
> 1) It sets a border radius for the <body> element, which leads to clipped
> corners on platforms where we don't use rounded popups.
> 2) It uses a white background for its <body> element, which is what we use
> for the arrow.
> 
> So, while those issues should be fixed, they would need to be fixed in the
> add-on itself.

I created a minimal demo add-on, with a popup.html which links a css with only

body { background-color: #5f5f5f; }

Yet I still get this problem with Nightly 53.0a1 built today.  Is it a regression?  I've got no border radius on body, background on body isn't white, everything else is default.

If there are specific hoops to jump through to get this fix to work, such that a very basic CSS statement to modify background color fails to "trigger" the fix, then it's not a robust fix, and those hoops need to be documented if a proper fix can't be found.

Comment 21

[Kris Maglione [:kmag]]

It works as expected for me. Can you attach your demo add-on?

Comment 22

[Leif-AMO]

Attachment: popup.xpi

This is a very minimalist demo.  I include a locale, and a javascript to find and replace the messages in page.  I include a bunch of icon sizes.

Other than that, I am doing almost nothing in CSS.

I have tried with true and false flags set for manifest.json -> browser_action -> browser_style , same result either way.

Use testing:

1) Install add-on, or unzip files and run with web-ext.
2) Click "POP^UP" icon, see mismatch in style.  (Screenshot on bug 1331999 (duplicate) https://bug1331999.bmoattachments.org/attachment.cgi?id=8828005 )

Comment 23

[Kris Maglione [:kmag]]

The add-on works as expected for me. Can you provide more details about your system? The contents of about:support would be particularly useful.

Also, are you testing on an official nightly or a custom build?

Comment 24

[Leif-AMO]

I have had this problem for as long as I started learning web extensions, last June, about version 46 (release).  Took a break from WebExtensions until November, been using nightlies since, and have always had this.

OS Windows 7 AMD x64 SP1, most* updates.  Multiple versions of Firefox installed in parallel, each version using it's own install folder ("C:\Program Files\Mozilla\Firefox\<version>") and each with it's own clean profile (delete old profile manually when uninstalling, create new when installing, update shortcuts to launch each bin/profile).  The extension is loaded from running web-ext within the add-on directory, and also from using 7-Zip to create an .xpi, with an .ignore to ignore errata.

Downloaded before filing bug:

http://releases.mozilla.org/pub/firefox/nightly/2017/01/2017-01-18-03-02-14-mozilla-central/firefox-53.0a1.en-US.win64.installer.exe

If there's any other info you need, let me know, I will dig it up.  Thanks for taking a look.

about:support log:

{
  "application": {
    "name": "Firefox",
    "osVersion": "Windows_NT 6.1",
    "version": "53.0a1",
    "buildID": "20170118030214",
    "userAgent": "Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0",
    "safeMode": false,
    "updateChannel": "nightly",
    "supportURL": "https://support.mozilla.org/1/firefox/53.0a1/WINNT/en-US/",
    "numTotalWindows": 1,
    "numRemoteWindows": 1,
    "remoteAutoStart": true,
    "autoStartStatus": 1
  },
  "modifiedPreferences": {
    "browser.cache.frecency_experiment": 1,
    "browser.cache.disk.smart_size.first_run": false,
    "browser.cache.disk.capacity": 1048576,
    "browser.cache.disk.filesystem_reported": 1,
    "browser.download.importedFromSqlite": true,
    "browser.download.manager.showWhenStarting": false,
    "browser.link.open_newwindow": 2,
    "browser.places.smartBookmarksVersion": 8,
    "browser.search.update": false,
    "browser.sessionstore.resume_from_crash": false,
    "browser.startup.homepage": "about:blank",
    "browser.startup.homepage_override.buildID": "20170118030214",
    "browser.startup.homepage_override.mstone": "53.0a1",
    "browser.tabs.warnOnClose": false,
    "browser.tabs.warnOnOpen": false,
    "browser.urlbar.lastSuggestionsPromptDate": 20170118,
    "browser.urlbar.daysBeforeHidingSuggestionsPrompt": 3,
    "dom.max_script_run_time": 30,
    "dom.gamepad.extensions.enabled": true,
    "dom.disable_open_during_load": false,
    "extensions.checkCompatibility.nightly": false,
    "extensions.lastAppVersion": "53.0a1",
    "javascript.options.showInConsole": true,
    "javascript.options.strict": true,
    "media.gmp.storage.version.observed": 1,
    "media.hardware-video-decoding.failed": false,
    "network.cookie.prefsMigrated": true,
    "network.manage-offline-status": false,
    "network.http.max-connections-per-server": 10,
    "network.http.phishy-userpass-length": 255,
    "places.history.expiration.transient_current_max_pages": 120530,
    "plugin.disable_full_page_plugin_for_types": "application/pdf",
    "security.warn_entering_weak": false,
    "security.warn_submit_insecure": false,
    "security.warn_viewing_mixed": false,
    "security.warn_entering_weak.show_once": false,
    "security.warn_entering_secure.show_once": false,
    "security.warn_leaving_secure": false,
    "security.warn_viewing_mixed.show_once": false,
    "security.fileuri.origin_policy": 3,
    "security.sandbox.content.tempDirSuffix": "{897219bd-5f39-4d5f-9632-036e63e12445}",
    "security.warn_leaving_secure.show_once": false,
    "security.warn_entering_secure": false,
    "security.fileuri.strict_origin_policy": false
  },
  "lockedPreferences": {},
  "javaScript": {
    "incrementalGCEnabled": true
  },
  "accessibility": {
    "isActive": false,
    "forceDisabled": 0
  },
  "libraryVersions": {
    "NSPR": {
      "minVersion": "4.13.1",
      "version": "4.13.1"
    },
    "NSS": {
      "minVersion": "3.29 Beta",
      "version": "3.29 Beta"
    },
    "NSSUTIL": {
      "minVersion": "3.29 Beta",
      "version": "3.29 Beta"
    },
    "NSSSSL": {
      "minVersion": "3.29 Beta",
      "version": "3.29 Beta"
    },
    "NSSSMIME": {
      "minVersion": "3.29 Beta",
      "version": "3.29 Beta"
    }
  },
  "userJS": {
    "exists": true
  },
  "crashes": {
    "submitted": [],
    "pending": 0
  },
  "sandbox": {
    "contentSandboxLevel": 2
  },
  "experiments": [],
  "extensions": [
    {
      "name": "Application Update Service Helper",
      "version": "1.0",
      "isActive": true,
      "id": "[e-mail]"
    },
    {
      "name": "FlyWeb",
      "version": "1.0.0",
      "isActive": true,
      "id": "[e-mail]"
    },
    {
      "name": "Form Autofill",
      "version": "1.0",
      "isActive": true,
      "id": "[e-mail]"
    },
    {
      "name": "Multi-process staged rollout",
      "version": "1.7",
      "isActive": true,
      "id": "[e-mail]"
    },
    {
      "name": "Pocket",
      "version": "1.0.5",
      "isActive": true,
      "id": "[e-mail]"
    },
    {
      "name": "Popup",
      "version": "0.0.0.1",
      "isActive": true,
      "id": "{01234567-ABCD-ABCD-ABCD-0123456789AB}"
    },
    {
      "name": "Presentation",
      "version": "1.0.0",
      "isActive": true,
      "id": "[e-mail]"
    },
    {
      "name": "Shield Recipe Client",
      "version": "1.0.0",
      "isActive": true,
      "id": "[e-mail]"
    },
    {
      "name": "Web Compat",
      "version": "1.0",
      "isActive": true,
      "id": "[e-mail]"
    }
  ],
  "graphics": {
    "numTotalWindows": 2,
    "numAcceleratedWindows": 2,
    "windowLayerManagerType": "Direct3D 11",
    "windowLayerManagerRemote": true,
    "supportsHardwareH264": "Yes; Using D3D9 API",
    "currentAudioBackend": "wasapi",
    "adapterDescription": "NVIDIA GeForce GTX 480",
    "adapterVendorID": "0x10de",
    "adapterDeviceID": "0x06c0",
    "adapterSubsysID": "14803842",
    "adapterRAM": "1536",
    "adapterDrivers": "nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um",
    "driverVersion": "21.21.13.7306",
    "driverDate": "10-1-2016",
    "adapterDescription2": "",
    "adapterVendorID2": "",
    "adapterDeviceID2": "",
    "adapterSubsysID2": "",
    "adapterRAM2": "",
    "adapterDrivers2": "",
    "driverVersion2": "",
    "driverDate2": "",
    "isGPU2Active": false,
    "direct2DEnabled": true,
    "directWriteEnabled": true,
    "directWriteVersion": "6.2.9200.21976",
    "clearTypeParameters": "Gamma: 2.2 Pixel Structure: BGR ClearType Level: 50 Enhanced Contrast: 100 ",
    "webglRenderer": "Google Inc. -- ANGLE (NVIDIA GeForce GTX 480 Direct3D11 vs_5_0 ps_5_0)",
    "webgl2Renderer": "Google Inc. -- ANGLE (NVIDIA GeForce GTX 480 Direct3D11 vs_5_0 ps_5_0)",
    "info": {
      "AzureCanvasBackend": "skia",
      "AzureCanvasAccelerated": 0,
      "AzureFallbackCanvasBackend": "cairo",
      "AzureContentBackend": "skia",
      "ApzWheelInput": 1,
      "ApzTouchInput": 1,
      "ApzDragInput": 1
    },
    "featureLog": {
      "features": [
        {
          "name": "HW_COMPOSITING",
          "description": "Compositing",
          "status": "available",
          "log": [
            {
              "type": "default",
              "status": "available"
            }
          ]
        },
        {
          "name": "D3D11_COMPOSITING",
          "description": "Direct3D11 Compositing",
          "status": "available",
          "log": [
            {
              "type": "default",
              "status": "available"
            }
          ]
        },
        {
          "name": "D3D9_COMPOSITING",
          "description": "Direct3D9 Compositing",
          "status": "disabled",
          "log": [
            {
              "type": "default",
              "status": "disabled",
              "message": "Disabled by default"
            }
          ]
        },
        {
          "name": "DIRECT2D",
          "description": "Direct2D",
          "status": "available",
          "log": [
            {
              "type": "default",
              "status": "available"
            }
          ]
        },
        {
          "name": "D3D11_HW_ANGLE",
          "description": "Direct3D11 hardware ANGLE",
          "status": "available",
          "log": [
            {
              "type": "default",
              "status": "available"
            }
          ]
        },
        {
          "name": "GPU_PROCESS",
          "description": "GPU Process",
          "status": "available",
          "log": [
            {
              "type": "default",
              "status": "available"
            }
          ]
        }
      ],
      "fallbacks": []
    },
    "crashGuards": []
  }
}