Closed Bug 1478391 Opened Last year Closed Last year

Move Appearance to not use mako.

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(3 files)

This will make bug 1428676 much easier.

It involves changing appearance to be an enum class while at it.
Assignee: nobody → emilio
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.
Jonathan, mind helping out to fix the Windows / Mac build if you have the time?
Flags: needinfo?(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.
Flags: needinfo?(jwatt)
Attached patch mac fixesSplinter Review
Here are the fixes you need to roll up into your patch to get it to work on mac.
Attached patch windows fixesSplinter Review
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 on attachment 8994874 [details]
Bug 1478391: Autogenerate StyleAppearance. r=jwatt

Jonathan Watt [:jwatt] has approved the revision.

https://phabricator.services.mozilla.com/D2361
Attachment #8994874 - Flags: review+
(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 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 :)
Oops. Can you tell my Windows machine isn't exactly set up right currently?
https://hg.mozilla.org/mozilla-central/rev/ced9b3994cf4
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1479216
Duplicate of this bug: 1371809
Duplicate of this bug: 1371811
You need to log in before you can comment on or make changes to this bug.