1508154 - Rename toolbar_bottom_separator to reflect what it actually does

Status: NEW

(WebExtensions :: Themes, enhancement, P3)

Description

[Tim Nguyen :ntim]

Since bug 1468517, the toolbar_bottom_separator color is also applied as a top border color for the findbar, so the "bottom" part of the name is wrong.

A more semantic name like "chrome_content_separator" would be a better choice.

Problem is, there are already add-ons using the toolbar_bottom_separator name. We could alias the name, but generally aliases tend to get messy. Or simply rename and mass migrate existing themes.

Comment 1

[Tim Nguyen :ntim]

From Mike Conca on Slack:

As for bug 1508154, an alias is out of the question since the reason we're getting rid of the other aliases is to allow proper json validation without extra code to maintain.
To be honest, I'm not inclined to change the name.  Yeah, it is a bit wrong.  But so are a lot of the names.  And it feels like this could become an never ending process.
Since the name is released, I don't think it is worth the overhead of the announcement, changing firefox and linter to warnings, waiting 3 releases, then making the final changes.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #0)
> Since bug 1468517, the toolbar_bottom_separator color is also applied as a
> top border color for the findbar, so the "bottom" part of the name is wrong.
> 
> A more semantic name like "chrome_content_separator" would be a better
> choice.
> 
> Problem is, there are already add-ons using the toolbar_bottom_separator
> name. We could alias the name, but generally aliases tend to get messy.

Still seems better than being stuck with a misleading name forever.

> Or simply rename and mass migrate existing themes.

+1 if that's easy enough to do.

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #1)
> From Mike Conca on Slack:
> 
> As for bug 1508154, an alias is out of the question since the reason we're
> getting rid of the other aliases is to allow proper json validation without
> extra code to maintain.

Why is this a significant burden? What kind of overhead are we talking about and how does that trump making these themes more intuitive and easy to author?

> Since the name is released, I don't think it is worth the overhead of the
> announcement, changing firefox and linter to warnings, waiting 3 releases,
> then making the final changes.

I don't think we need a big announcement, but a note somewhere in the release notes might be a good idea. I can't speak for the linter stuff but the Firefox changes are going to be straightforward.

There's a bunch of other property names that have proven to be confusing (accentcolor) or are inconsistent (e.g. camel case vs. underscores), so it would be nice if we could establish a workable path forward here for the sake of providing a sane theming API. We could even bundle a bunch of changes if that makes things easier.

Comment 3

[Mike Conca [:mconca]]

(In reply to Dão Gottwald [::dao] from comment #2)
> (In reply to Tim Nguyen :ntim (please use needinfo?) from comment #0)
> > Since bug 1468517, the toolbar_bottom_separator color is also applied as a
> > top border color for the findbar, so the "bottom" part of the name is wrong.
> > 
> > A more semantic name like "chrome_content_separator" would be a better
> > choice.
> > 
> > Problem is, there are already add-ons using the toolbar_bottom_separator
> > name. We could alias the name, but generally aliases tend to get messy.
> 
> Still seems better than being stuck with a misleading name forever.
> 
> > Or simply rename and mass migrate existing themes.
> 
> +1 if that's easy enough to do.
> 
> (In reply to Tim Nguyen :ntim (please use needinfo?) from comment #1)
> > From Mike Conca on Slack:
> > 
> > As for bug 1508154, an alias is out of the question since the reason we're
> > getting rid of the other aliases is to allow proper json validation without
> > extra code to maintain.
> 
> Why is this a significant burden? What kind of overhead are we talking about
> and how does that trump making these themes more intuitive and easy to
> author?

See https://bugzilla.mozilla.org/show_bug.cgi?id=1472740#c0

> > Since the name is released, I don't think it is worth the overhead of the
> > announcement, changing firefox and linter to warnings, waiting 3 releases,
> > then making the final changes.
> 
> I don't think we need a big announcement, but a note somewhere in the
> release notes might be a good idea. I can't speak for the linter stuff but
> the Firefox changes are going to be straightforward.

We have a defined process that is followed whenever we deprecate any part of the WebExtensions API.  See https://m.wiki.mozilla.org/WebExtensions/DeprecationPolicy.

> There's a bunch of other property names that have proven to be confusing
> (accentcolor) or are inconsistent (e.g. camel case vs. underscores), so it
> would be nice if we could establish a workable path forward here for the
> sake of providing a sane theming API. We could even bundle a bunch of
> changes if that makes things easier.

If cleaning up the theme API is a goal, we should follow a deliberate process. Reviewing all existing names against a proposed, standardized naming scheme and then collecting those changes below a meta bug would be ideal.

Comment 4

[Dão Gottwald [:dao]]

(In reply to Mike Conca [:mconca] from comment #3)
> > (In reply to Tim Nguyen :ntim (please use needinfo?) from comment #1)
> > > From Mike Conca on Slack:
> > > 
> > > As for bug 1508154, an alias is out of the question since the reason we're
> > > getting rid of the other aliases is to allow proper json validation without
> > > extra code to maintain.
> > 
> > Why is this a significant burden? What kind of overhead are we talking about
> > and how does that trump making these themes more intuitive and easy to
> > author?
> 
> See https://bugzilla.mozilla.org/show_bug.cgi?id=1472740#c0

Not all of these points apply here. The aliases would exist for backwards compatibility, so we should in fact not document them, as we don't want people to use them going forward. The goal is to make themes easier to author.

I understand this adds some complexity for validation, but this seems like a price worth paying. At least I don't see yet why this would be a showstopper.

And again, I'm not dead set on aliasing. If we could mass-migrate existing themes on AMO that might indeed be the better option.

I'm gonna morph this bug to better reflect the motivation here and ask for this to be re-prioritized.