629474 - Addon ID generated for lightweight themes is not correct

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Mike Kaply [:mkaply]]

Currently there is code in LightweightThemeManager.jsm to generate an addon ID. It looks like this:

function _getInternalID(id) {
  if (!id)
    return null;
  let len = id.length - ID_SUFFIX.length;
  if (len > 0 && id.substring(len) == ID_SUFFIX)
    return id.substring(0, len);
  return null;
}

where ID_SUFFIX is @personas.mozilla.org

While this works for personas from getpersonas.com (and I assume AMO) where the IDs are simply numbers, anyone using a correct extension ID gets this for an ID:

[e-mail]@personas.mozilla.org

The ID code should not be adding on @personas.mozilla.org if it sees an @ in the output.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Wouldn't such a change break the patch in bug 580298?

Comment 2

[Mike Kaply [:mkaply]]

Is looking at the ID of the addon really the only way to tell if something is a lightweight theme?

That looks like a bad patch for me.

I'm really disappointed that we're putting code throughout Firefox that assumes that the IDs of themes are just sets of digits or single words.

Should theme IDs be normal IDs? 

Just because getpersonas.com screwed up in the beginning doesn't mean we should be following their lead.

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #2)
> Is looking at the ID of the addon really the only way to tell if something is a
> lightweight theme?

I think so, yea (from the UI, at least). In the future they'll probably be split by the type property ("theme" vs" lwtheme") - but its too risky to do before 4.0. So its probably best to leave fixing this bug til after 4.0 too.

> Should theme IDs be normal IDs? 

It shouldn't matter, as long as they're unique. Lightweight themes are currently the only addon type that it makes a difference for - and hopefully that requirement will disappear too.

Comment 4

[Mike Kaply [:mkaply]]

This isn't worth fixing. It doesn't break anything.