Closed Bug 629474 Opened 13 years ago Closed 7 years ago

Addon ID generated for lightweight themes is not correct

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mkaply, Unassigned)

Details

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:

moviepremierepersona@brandthunder.com@personas.mozilla.org

The ID code should not be adding on @personas.mozilla.org if it sees an @ in the output.
Wouldn't such a change break the patch in bug 580298?
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.
(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.
This isn't worth fixing. It doesn't break anything.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.