Closed Bug 1427107 Opened 2 years ago Closed 2 years ago

Invalid badge background color behaves as null

Categories

(WebExtensions :: Frontend, defect, P5)

defect

Tracking

(firefox59 verified, firefox60 verified)

VERIFIED FIXED
mozilla59
Tracking Status
firefox59 --- verified
firefox60 --- verified

People

(Reporter: Oriol, Assigned: Oriol)

Details

Attachments

(3 files, 1 obsolete file)

Attached file test.xpi (obsolete) —
Run this code:

  browser.browserAction.setBadgeBackgroundColor({color: "red"});
  browser.browserAction.setBadgeBackgroundColor({tabId, color: "green"});
  browser.browserAction.setBadgeBackgroundColor({tabId, color: "invalid"});

Since "invalid" is not a color, it behaves as null, i.e. it removes the tab-specific green color and the tab inherits the global red color.

However, Chrome behaves differently. There the "invalid" color is ignored, so the tab continues having a green tab-specific color. And a warning is logged.

I think compatibility with Chrome should be preferred, if someone wants to remove a tab-specific color, since bug 1419940 they can explicitly use null, which is less hacky than an invalid color.
Attached file test.xpi
There was a mistake in the attached test webext
Attachment #8938893 - Attachment is obsolete: true
Priority: -- → P5
Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED
Summary: Invalid badge background color behaves an null → Invalid badge background color behaves as null
Comment on attachment 8941904 [details]
Bug 1427107 - Reject setBadgeBackgroundColor promise for invalid string colors

https://reviewboard.mozilla.org/r/212120/#review218914
Attachment #8941904 - Flags: review?(mixedpuppy) → review+
Keywords: checkin-needed
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s ee783915e508a3257642d4fc4eb183b020963b09 -d 7be74a94e766: rebasing 442801:ee783915e508 "Bug 1427107 - Reject setBadgeBackgroundColor promise for invalid string colors r=mixedpuppy" (tip)
merging browser/components/extensions/ext-browserAction.js
merging browser/components/extensions/test/browser/browser_ext_browserAction_context.js
warning: conflicts while merging browser/components/extensions/ext-browserAction.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
The code base has been updated and the patch fails to apply now. Please push fix the issue and push the updated patch to review board. Thank you.
Flags: needinfo?(oriol-bugzilla)
Keywords: checkin-needed
Done!
Flags: needinfo?(oriol-bugzilla)
Keywords: checkin-needed
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bc54bd9b08c8
Reject setBadgeBackgroundColor promise for invalid string colors r=mixedpuppy
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bc54bd9b08c8
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Tested with the latest build and the only difference I see from before the fix is that the color of the "abcd" background is red by default and in the latest fix it's green. Is this the desired behavior along with the "Reading manifest: Error processing background.persistent: Event pages are not currently supported. This will run as a persistent background page." warning?
Flags: needinfo?(oriol-bugzilla)
Yes, the desired behavior is that the badge background color should be green on the tab where you installed the extension, just like Chrome. The event pages warning is unrelated.
Flags: needinfo?(oriol-bugzilla)
As per https://bugzilla.mozilla.org/show_bug.cgi?id=1427107#c10 setting this issue to verified on Windows 10 64 Bit Firefox 60.0a1 (20180123102017).
Status: RESOLVED → VERIFIED
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.