1478391 - Move Appearance to not use mako.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

This will make bug 1428676 much easier.

It involves changing appearance to be an enum class while at it.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1478391: Autogenerate StyleAppearance. r=jwatt

This builds on bug 1428676 and introduces StyleAppearance, which replaces the
NS_THEME_* constants.

Really sorry for the size of the patch.

There's a non-trivial change in the gtk theme, which I submitted separately as
bug 1478385.

Most of it is automated with the following script:

```python
import sys
import re

CAMEL_CASE_REGEX = re.compile(r"(^|_|-)([A-Z])([A-Z]+)")
THEME_REGEX = re.compile(r"\bNS_THEME_([A-Z_]+)\b")

def to_camel_case(ident):
  return re.sub(CAMEL_CASE_REGEX,
                lambda m: m.group(2) + m.group(3).lower(), ident)

def constant_to_enum(constant):
  return "StyleAppearance::" + to_camel_case(constant)

def process_line(line):
  return re.sub(THEME_REGEX,
                lambda m: constant_to_enum(m.group(1)), line)

lines = []
with open(sys.argv[1], "r") as f:
  for line in f:
    lines.append(process_line(line))

with open(sys.argv[1], "w") as f:
  for line in lines:
    f.write(line)
```

But still I suspect Windows and Mac won't be building. I'd need a bit of help
with that.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Jonathan, mind helping out to fix the Windows / Mac build if you have the time?

Comment 3

[Jonathan Watt [:jwatt]]

Can do for Mac. My Windows machine isn't managing to complete a build ATM though, which I've not managed to fix yet. I'm at the point of giving up on that an reinstalling Windows and everything from scratch, in which case I can take a look at that platform tomorrow.

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: mac fixes

Here are the fixes you need to roll up into your patch to get it to work on mac.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a38b07bd5508d91046fd2214719479252852933

Green on Linux so far, will push to try with the mac fixes later.

Comment 6

[Jonathan Watt [:jwatt]]

Attachment: windows fixes

Note the int() casts in two places and the corresponding comment that I added to specified/box.rs. If you don't like that, you may want to do something similar to the changes I made to the block of code in nsNativeThemeWin::GetThemePartAndState beginning:

    case StyleAppearance::Statusbarpanel:
    case StyleAppearance::Resizerpanel:
    case StyleAppearance::Resizer: {

Comment 7

[Phabricator Automation]

Comment on attachment 8994874 [details]
Bug 1478391: Autogenerate StyleAppearance. r=jwatt

Jonathan Watt [:jwatt] has approved the revision.

https://phabricator.services.mozilla.com/D2361

Comment 8

[Emilio Cobos Álvarez (:emilio)]

(In reply to Jonathan Watt [:jwatt] from comment #6)
> Created attachment 8994962 [details] [diff] [review]
> windows fixes
> 
> Note the int() casts in two places and the corresponding comment that I
> added to specified/box.rs. If you don't like that, you may want to do
> something similar to the changes I made to the block of code in
> nsNativeThemeWin::GetThemePartAndState beginning:
> 
>     case StyleAppearance::Statusbarpanel:
>     case StyleAppearance::Resizerpanel:
>     case StyleAppearance::Resizer: {

Yeah, the changes and the comment look good, thanks!

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8994962 [details] [diff] [review]
windows fixes

Review of attachment 8994962 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsNativeThemeWin.cpp
@@ +1197,5 @@
>      case StyleAppearance::Statusbarpanel:
>      case StyleAppearance::Resizerpanel:
>      case StyleAppearance::Resizer: {
> +	  switch (aWidgetType) {
> +		case StyleAppearance::Statusbarpanel:

I'll fix the tabs usage here and in another couple places though :)

Comment 10

[Jonathan Watt [:jwatt]]

Oops. Can you tell my Windows machine isn't exactly set up right currently?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ced9b3994cf4
Autogenerate StyleAppearance. r=jwatt

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ced9b3994cf4