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 Facebook homepage sign up inputs have extra padding #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...

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...

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...

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...

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...

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...

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...

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 The changes caused existing tests to fail. label Jan 17, 2018

ferjm force-pushed the issue-18831-fb-layout branch from eb92be8 to 2309d99 Compare January 17, 2018 15:35

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 17, 2018

highfive added the S-tests-failed The changes caused existing tests to fail. label Jan 17, 2018

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

highfive removed the S-tests-failed The changes caused existing tests to fail. label Jan 17, 2018

highfive added the S-tests-failed The changes caused existing tests to fail. label Jan 17, 2018

mbrubeck approved these changes Jan 18, 2018

View reviewed changes

mbrubeck removed the S-awaiting-review There is new code that needs to be reviewed. label Jan 18, 2018

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

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 19, 2018

highfive removed the S-tests-failed The changes caused existing tests to fail. 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 The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 22, 2018

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 22, 2018

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 22, 2018

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

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. 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 issue-18831-fb-layout branch February 14, 2018 08:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Loading

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

Loading

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

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 Loading

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

Choose a reason for hiding this 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 Loading

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

Loading

emilio commented Jan 22, 2018 •

edited

Loading