mozilla / positron

a experimental, Electron-compatible runtime on top of Gecko
Other
562 stars 64 forks source link

support display: -webkit-flex and -webkit-flex: 1 #75

Closed mykmelez closed 8 years ago

mykmelez commented 8 years ago

@brendandahl: This should enable display: -webkit-flex and -webkit-flex: 1, and it does fix the layout of the navigation bar in the sample browser, but the iframe overlaps the navigation bar, partially obscuring it, when the app is first started:

screen shot 2016-06-07 at 10 41 24

That problem goes away (and stays away) as soon as I resize the app window. Another problem is a horizontal scrollbar at the bottom of the app window, which is persistent, regardless of window size.

I'm not sure if either of these are problems with this branch in particular, Gecko's support for flex in general, or perhaps the app itself.

brendandahl commented 8 years ago

Perhaps we need -webkit-flex-direction too?

brendandahl commented 8 years ago

I haven't check it after all of our changes, but I was previously able to get it to display correctly by doing:

65c65
<   -webkit-box-sizing: border-box;
---
>   box-sizing: border-box;
69,70c69
<   display: -webkit-flex;
<   -webit-flex-direction: column;
---
>   display: flex;
74,76c73,74
<   -webkit-flex: 1;
<   display: -webkit-flex;
<   -webit-flex-direction: column;
---
>   flex: 1;
>   display: flex;
80c78
<   -webkit-flex: 1;
---
>   flex: 1;
101,103c99,101
<   -webkit-flex: 1;
<   display: -webkit-flex;
<   -webit-flex-direction: row;
---
>   flex: 1;
>   display: flex;
>   flex-direction: row;
117,118c115,116
<   -webkit-box-sizing: border-box;
<   -webkit-flex: 1;
---
>   box-sizing: border-box;
>   flex: 1;
239d236
< 
brendandahl commented 8 years ago

Actually this branch+master seems to good for me. (no resizing) https://www.dropbox.com/s/semd3ir55po4t6m/Screenshot%202016-06-07%2011.53.32.png?dl=0

mykmelez commented 8 years ago

Perhaps we need -webkit-flex-direction too?

I was going to do that but then noticed that the app actually misspells all its -webkit-flex-direction properties as -webit-flex-direction, so those shouldn't be applied by in Chromium either.

mykmelez commented 8 years ago

Actually this branch+master seems to good for me. (no resizing)

Is that with or without your modifications to the app itself?

mykmelez commented 8 years ago

I haven't check it after all of our changes, but I was previously able to get it to display correctly by doing:

It looks like these changes only set flex-direction: row, but that's the initial value of the property, per https://developer.mozilla.org/en-US/docs/Web/CSS/flex-direction, so I would think they'd need to do the opposite and set flex-direction: column when needed.

brendandahl commented 8 years ago

Is that with or without your modifications to the app itself?

Without my modifications.

mykmelez commented 8 years ago

Without my modifications.

Hunh, strange, I wonder why you and I are seeing different results!

mykmelez commented 8 years ago

@brendandahl found out from @dholbert that these aliases landed recently in mozilla-central, so #78 merged from upstream to acquire them.

mykmelez commented 8 years ago

FWIW, I still see the first-paint issue after merging from upstream. I filed https://github.com/mozilla/positron/issues/79 on it.