Do not leave space below baseline when it is not needed (i.e.there is… #19789

ferjm · 2018-01-17T13:09:49Z

… no text)

This is my first layout fix and a naive approach to fix #18831.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #18831

This change is

ferjm · 2018-01-17T13:10:15Z

r? @mbrubeck

ferjm · 2018-01-17T13:10:50Z

@bors-servo try

bors-servo · 2018-01-17T13:10:54Z

⌛ Trying commit eb92be8 with merge 34182e2...


        Auto merge of #19789 - ferjm:issue-18831-fb-layout, r=<try>

Do not leave space below baseline when it is not needed (i.e.there is… … no text) This is my first layout fix and a naive approach to fix #18831. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #18831  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19789)

bors-servo · 2018-01-17T13:41:14Z

💔 Test failed - linux-rel-wpt

ferjm · 2018-01-17T15:39:58Z

@bors-servo try

bors-servo · 2018-01-17T15:40:02Z

⌛ Trying commit 2309d99 with merge efbf6ef...


        Auto merge of #19789 - ferjm:issue-18831-fb-layout, r=<try>

Do not leave space below baseline when it is not needed (i.e.there is… … no text) This is my first layout fix and a naive approach to fix #18831. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #18831  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19789)

bors-servo · 2018-01-17T16:08:56Z

💔 Test failed - linux-rel-wpt

ferjm · 2018-01-17T16:12:55Z

@bors-servo try

bors-servo · 2018-01-17T16:13:02Z

⌛ Trying commit 3df5533 with merge 8b25e42...


        Auto merge of #19789 - ferjm:issue-18831-fb-layout, r=<try>

Do not leave space below baseline when it is not needed (i.e.there is… … no text) This is my first layout fix and a naive approach to fix #18831. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #18831  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19789)

bors-servo · 2018-01-17T16:53:00Z

💔 Test failed - linux-rel-css

tigercosmos · 2018-01-17T19:30:29Z

{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/css/CSS2/lists/list-style-type-applies-to-012.xht", "line": 10508, "action": "test_result", "expected": "FAIL"}
{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/css/CSS2/lists/list-style-applies-to-012.xht", "line": 13721, "action": "test_result", "expected": "FAIL"}
{"status": "FAIL", "group": "default", "message": "/css/css-flexbox/flexbox_inline.html b67f1e6f6b6c3832ad0fbfd3ae2560354ecaf46b\n/css/css-flexbox/flexbox_inline-ref.html 37bdb4e7e846e523fecba0231ebac2333842ca72\nTesting b67f1e6f6b6c3832ad0fbfd3ae2560354ecaf46b == 37bdb4e7e846e523fecba0231ebac2333842ca72", "stack": null, "subtest": null, "test": "/css/css-flexbox/flexbox_inline.html", "line": 19898, "action": "test_result", "expected": "PASS"}

mbrubeck

This looks good to me, but I'm curious about the failing test. If it's something that was passing "by accident" (i.e. it is failing for an unrelated reason), feel free to update test expectations and land with r=mbrubeck, and file a follow-up issue if necessary.

mbrubeck · 2018-01-18T19:28:15Z

The failing test involves an inline-flex box that contains text content that generates an anonymous box. It seems like the text within the inline-flex box should have its line metrics calculated separately from the text in its container, but it isn't for some reason.

I'd prefer to file a separate issue for this, and land the current patch as-is.

ferjm · 2018-01-19T11:21:58Z

@bors-servo try

ferjm · 2018-01-22T10:41:48Z

@bors-servo try

bors-servo · 2018-01-22T10:41:52Z

⌛ Trying commit a241bed with merge b65dbb4...


        Auto merge of #19789 - ferjm:issue-18831-fb-layout, r=<try>

Do not leave space below baseline when it is not needed (i.e.there is… … no text) This is my first layout fix and a naive approach to fix #18831. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #18831  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19789)

bors-servo · 2018-01-22T11:30:30Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True

emilio · 2018-01-22T12:33:35Z

@bors-servo r=mbrubeck delegate+

bors-servo · 2018-01-22T12:33:36Z

📌 Commit a241bed has been approved by mbrubeck

bors-servo · 2018-01-22T12:33:40Z

⌛ Testing commit a241bed with merge 90f46c0...


        Auto merge of #19789 - ferjm:issue-18831-fb-layout, r=mbrubeck

Do not leave space below baseline when it is not needed (i.e.there is… … no text) This is my first layout fix and a naive approach to fix #18831. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #18831  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19789)

bors-servo · 2018-01-22T12:33:45Z

✌️ @ferjm can now approve this pull request

bors-servo · 2018-01-22T12:33:46Z

💡 This pull request was already approved, no need to approve it again.

This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

bors-servo · 2018-01-22T12:33:46Z

📌 Commit a241bed has been approved by mbrubeck

bors-servo · 2018-01-22T12:33:51Z

⌛ Testing commit a241bed with merge 580f0d7...


        Auto merge of #19789 - ferjm:issue-18831-fb-layout, r=mbrubeck

Do not leave space below baseline when it is not needed (i.e.there is… … no text) This is my first layout fix and a naive approach to fix #18831. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #18831  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19789)

bors-servo · 2018-01-22T13:02:35Z

💔 Test failed - linux-rel-wpt

ferjm · 2018-01-22T13:13:47Z

@bors-servo retry

bors-servo · 2018-01-22T14:14:31Z

⌛ Testing commit a241bed with merge ec8975b...


        Auto merge of #19789 - ferjm:issue-18831-fb-layout, r=mbrubeck

Do not leave space below baseline when it is not needed (i.e.there is… … no text) This is my first layout fix and a naive approach to fix #18831. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #18831  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19789)

bors-servo · 2018-01-22T15:08:21Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: mbrubeck
Pushing ec8975b to master...

highfive assigned mbrubeck Jan 17, 2018

highfive added the S-tests-failed label Jan 17, 2018

ferjm force-pushed the ferjm:issue-18831-fb-layout branch from eb92be8 to 2309d99 Jan 17, 2018

highfive added S-awaiting-review and removed S-tests-failed labels Jan 17, 2018

highfive added the S-tests-failed label Jan 17, 2018

ferjm force-pushed the ferjm:issue-18831-fb-layout branch from 2309d99 to 3df5533 Jan 17, 2018

highfive removed the S-tests-failed label Jan 17, 2018

highfive added the S-tests-failed label Jan 17, 2018

mbrubeck approved these changes Jan 18, 2018

View changes

mbrubeck removed the S-awaiting-review label Jan 18, 2018

ferjm force-pushed the ferjm:issue-18831-fb-layout branch from 3df5533 to 1b8e9df Jan 19, 2018

highfive added S-awaiting-review and removed S-tests-failed labels Jan 19, 2018

highfive removed the S-tests-failed label Jan 22, 2018

ferjm mentioned this pull request Jan 22, 2018

Investigate erroneous calculation of line metrics for text within an inline-flex box #19828

Open

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 22, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Jan 22, 2018

highfive added S-awaiting-merge and removed S-tests-failed labels Jan 22, 2018

bors-servo merged commit a241bed into servo:master Jan 22, 2018
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jan 22, 2018

ferjm mentioned this pull request Jan 23, 2018

Facebook homepage sign up inputs have extra padding #18831

Open

ferjm deleted the ferjm:issue-18831-fb-layout branch Feb 14, 2018

servo / servo

Do not leave space below baseline when it is not needed (i.e.there is… #19789

Do not leave space below baseline when it is not needed (i.e.there is… #19789

ferjm commented Jan 17, 2018 •

edited by SimonSapin

ferjm commented Jan 17, 2018

ferjm commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

ferjm commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

ferjm commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

tigercosmos commented Jan 17, 2018

mbrubeck left a comment

mbrubeck commented Jan 18, 2018

ferjm commented Jan 19, 2018

ferjm commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

emilio commented Jan 22, 2018 •

edited

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

ferjm commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

servo / servo

Join GitHub today

Do not leave space below baseline when it is not needed (i.e.there is… #19789

Do not leave space below baseline when it is not needed (i.e.there is… #19789

Conversation

ferjm commented Jan 17, 2018 • edited by SimonSapin

ferjm commented Jan 17, 2018

ferjm commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

ferjm commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

ferjm commented Jan 17, 2018

bors-servo commented Jan 17, 2018

bors-servo commented Jan 17, 2018

tigercosmos commented Jan 17, 2018

mbrubeck left a comment

mbrubeck commented Jan 18, 2018

ferjm commented Jan 19, 2018

ferjm commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

emilio commented Jan 22, 2018 • edited

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

ferjm commented Jan 22, 2018

bors-servo commented Jan 22, 2018

bors-servo commented Jan 22, 2018

ferjm commented Jan 17, 2018 •

edited by SimonSapin

emilio commented Jan 22, 2018 •

edited