1293099 - Extend popup background color to arrow

Assignee

Description

•

8 years ago

+++ 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.

Kris Maglione [:kmag]

Assignee

Comment 1

•

8 years ago

Attached file Bug 1293099: Fill panel arrow and arrow content with browser body's background color. ui-r=maritz (deleted) — Details

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

Attachment #8778708 - Flags: review?(bwinton)

Kris Maglione [:kmag]

Assignee

Comment 2

•

8 years ago

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

Attachment #8778708 - Flags: ui-review?(mjaritz)

Markus Jaritz [:designakt] (UX)

Comment 3

•

8 years ago

Looks great. How does it look in Windows?

Flags: needinfo?(kmaglione+bmo)

Blake Winton (:bwinton) (:☕️)

Comment 4

•

8 years ago

mozreview-review

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!

Attachment #8778708 - Flags: review?(bwinton) → review+

Kris Maglione [:kmag]

Assignee

Comment 5

•

8 years ago

mozreview-review-reply

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.

Kris Maglione [:kmag]

Assignee

Comment 6

•

8 years ago

(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.

Kris Maglione [:kmag]

Assignee

Updated

•

8 years ago

Flags: needinfo?(kmaglione+bmo)

Comment hidden (mozreview-request)

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/

Kris Maglione [:kmag]

Assignee

Comment 8

•

8 years ago

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.

Attachment #8778708 - Flags: review+ → review?(bwinton)

Markus Jaritz [:designakt] (UX)

Comment 9

•

8 years ago

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?

Attachment #8778708 - Flags: ui-review?(mjaritz) → ui-review+

Blake Winton (:bwinton) (:☕️)

Comment 10

•

8 years ago

mozreview-review

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.  :)

Attachment #8778708 - Flags: review?(bwinton) → review+

Kris Maglione [:kmag]

Assignee

Comment 11

•

8 years ago

(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.

Markus Jaritz [:designakt] (UX)

Comment 12

•

8 years ago

(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.

Flags: needinfo?(kmaglione+bmo)

Kris Maglione [:kmag]

Assignee

Comment 13

•

8 years ago

(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.

Flags: needinfo?(kmaglione+bmo)

Kris Maglione [:kmag]

Assignee

Comment 14

•

8 years ago

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

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

Comment 15

•

8 years ago

bugherder

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

Status: NEW → RESOLVED

Closed: 8 years ago

status-firefox51: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla51

Jared Wein [:jaws] (please needinfo? me)

Updated

•

8 years ago

Depends on: 1294983

Jared Wein [:jaws] (please needinfo? me)

Updated

•

8 years ago

Depends on: 1294474

Vasilica Tamas, QA [:vtamas]

Comment 16

•

8 years ago

Attached file black_menu_for_googletm-11.2.1-fx.xpi (deleted) — Details

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?

Flags: needinfo?(kmaglione+bmo)

Kris Maglione [:kmag]

Assignee

Comment 17

•

8 years ago

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.

Flags: needinfo?(kmaglione+bmo)

Vasilica Tamas, QA [:vtamas]

Comment 18

•

8 years ago

(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.

Status: RESOLVED → VERIFIED

status-firefox51: fixed → verified

Leif-AMO

Comment 20

•

8 years ago

(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.

Kris Maglione [:kmag]

Assignee

Comment 21

•

8 years ago

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

Leif-AMO

Comment 22

•

8 years ago

Attached file popup.xpi (deleted) — Details

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 )

Kris Maglione [:kmag]

Assignee

Comment 23

•

8 years ago

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?

Leif-AMO

Comment 24

•

8 years ago

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": "aushelper@mozilla.org"
    },
    {
      "name": "FlyWeb",
      "version": "1.0.0",
      "isActive": true,
      "id": "flyweb@mozilla.org"
    },
    {
      "name": "Form Autofill",
      "version": "1.0",
      "isActive": true,
      "id": "formautofill@mozilla.org"
    },
    {
      "name": "Multi-process staged rollout",
      "version": "1.7",
      "isActive": true,
      "id": "e10srollout@mozilla.org"
    },
    {
      "name": "Pocket",
      "version": "1.0.5",
      "isActive": true,
      "id": "firefox@getpocket.com"
    },
    {
      "name": "Popup",
      "version": "0.0.0.1",
      "isActive": true,
      "id": "{01234567-ABCD-ABCD-ABCD-0123456789AB}"
    },
    {
      "name": "Presentation",
      "version": "1.0.0",
      "isActive": true,
      "id": "presentation@mozilla.org"
    },
    {
      "name": "Shield Recipe Client",
      "version": "1.0.0",
      "isActive": true,
      "id": "shield-recipe-client@mozilla.org"
    },
    {
      "name": "Web Compat",
      "version": "1.0",
      "isActive": true,
      "id": "webcompat@mozilla.org"
    }
  ],
  "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": []
  }
}

BMO Automation

Updated

•

6 years ago

Product: Toolkit → WebExtensions

Bug 1293099: Fill panel arrow and arrow content with browser body's background color. ui-r=maritz 8 years ago Kris Maglione [:kmag] (deleted), text/x-review-board-request	bwinton : review+ designakt : ui-review+	Details
black_menu_for_googletm-11.2.1-fx.xpi 8 years ago Vasilica Tamas, QA [:vtamas] (deleted), application/zip		Details
popup.xpi 8 years ago Leif-AMO (deleted), application/x-xpinstall		Details