Open Bug 1542023 Opened 2 years ago Updated 1 year ago

browser.theme.getCurrent() no longer returns empty object (`{}`) when default theme is enabled

Categories

(WebExtensions :: Themes, defect, P3)

defect

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: ntim, Assigned: robwu)

References

(Regression)

Details

(Keywords: regression)

Now returns:

{
  "colors": null,
  "icons": null,
  "images": null,
  "properties": null
}

instead.

Regressed by: 1525762
Keywords: regression

The priority flag is not set for this bug.
:mikedeboer, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(mdeboer)
Flags: needinfo?(mdeboer)
Priority: -- → P2

I think browser.theme.getCurrent() should return the colours used by the default theme.

That way, it’d possible to write theme agnostic code without needing to hardcode Photon theme values.

(In reply to ExE Boss from comment #2)

I think browser.theme.getCurrent() should return the colours used by the default theme.

That way, it’d possible to write theme agnostic code without needing to hardcode Photon theme values.

This bug is just about restoring the old behaviour to avoid breaking add-ons, returning default theme colors is a different bug.

This is a defect compared to what it used to do, but what it used to do was different based on what the built-ins were.

Now that they're static themes, we should revisit the question of what they should return. I assume that's the call of the Front-end team?

Priority: P2 → P3
Blocks: 1581886

I checked all public add-ons on AMO, and none of them rely on the existence of the key.

They typically check with if (theme.colors) { instead of something like if ("colors" in theme) {. Given that, and the absence of other bug reports, I'm going to:

Assignee: nobody → rob
Status: NEW → ASSIGNED

Tim, does comment 5 sound like an acceptable approach?

Flags: needinfo?(ntim.bugs)

I guess the change in behaviour for theme.getCurrent() is fine if it doesn't break add-ons, but it'd be nice if we had a clear definition of the ideal behaviour that's not just a side effect of implementation.

Flags: needinfo?(ntim.bugs)
You need to log in before you can comment on or make changes to this bug.