Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

layout: Use the margin box for vertical positioning of `inline-block` fragments if `overflow` is not `visible` per CSS 2.1 § 10.8.1. #13732

Merged

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Oct 12, 2016

Additionally, this patch reverts the change introduced in #12642 in
favor of the spec-compliant behavior described above. This patch also
removes the inline_block_overflow.html reftest introduced in #3725, as
the behavior it expected contradicted CSS 2.1 (and in fact the test
fails in Gecko).

The changes that this patch makes to input_selection_a.html and
input_selection_incremental_a.html are necessary workarounds to make
the tests pass in light of the fact that Servo's UA stylesheet applies
overflow: hidden to <input> elements. I believe that the changes are
not necessary in other rendering engines because they hard-code
overflow: hidden-like behavior for <input> elements, while Servo
uses the actual CSS overflow: hidden behavior. As far as I can tell,
Servo's behavior is arguably more spec-compliant, but it remains to be
seen how Web compatible it is.

Improves the Google results pages.

Closes #13707.

r? @notriddle


This change is Reviewable

@@ -8,6 +8,9 @@
font: 16px sans-serif;
color: black;
background: rgba(176, 214, 255, 1.0);
overflow: hidden;
display: inline-block;
color: white;

This comment has been minimized.

@notriddle

notriddle Oct 12, 2016

Contributor

Remove the color:black declaration.

@pcwalton pcwalton force-pushed the pcwalton:inline-block-vertical-align-overflow branch from 937faa0 to 79de34b Oct 12, 2016
@pcwalton
Copy link
Contributor Author

pcwalton commented Oct 12, 2016

@notriddle Done. r?

@notriddle
Copy link
Contributor

notriddle commented Oct 12, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2016

📌 Commit 79de34b has been approved by notriddle

@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2016

Testing commit 79de34b with merge 576aa97...

bors-servo added a commit that referenced this pull request Oct 13, 2016
… r=notriddle

layout: Use the margin box for vertical positioning of `inline-block` fragments if `overflow` is not `visible` per CSS 2.1 § 10.8.1.

Additionally, this patch reverts the change introduced in #12642 in
favor of the spec-compliant behavior described above. This patch also
removes the `inline_block_overflow.html` reftest introduced in #3725, as
the behavior it expected contradicted CSS 2.1 (and in fact the test
fails in Gecko).

The changes that this patch makes to `input_selection_a.html` and
`input_selection_incremental_a.html` are necessary workarounds to make
the tests pass in light of the fact that Servo's UA stylesheet applies
`overflow: hidden` to `<input>` elements. I believe that the changes are
not necessary in other rendering engines because they hard-code
`overflow: hidden`-like behavior for `<input>` elements, while Servo
uses the actual CSS `overflow: hidden` behavior. As far as I can tell,
Servo's behavior is arguably more spec-compliant, but it remains to be
seen how Web compatible it is.

Improves the Google results pages.

Closes #13707.

r? @notriddle

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13732)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2016

💔 Test failed - linux-rel-wpt

@highfive
Copy link

highfive commented Oct 13, 2016

  ▶ Unexpected subtest result in /html/webappapis/scripting/event-loops/microtask_after_raf.html:
  └ PASS [expected FAIL] Microtask execute immediately after script
@jdm
Copy link
Member

jdm commented Oct 13, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2016

Testing commit 79de34b with merge d465940...

bors-servo added a commit that referenced this pull request Oct 13, 2016
… r=notriddle

layout: Use the margin box for vertical positioning of `inline-block` fragments if `overflow` is not `visible` per CSS 2.1 § 10.8.1.

Additionally, this patch reverts the change introduced in #12642 in
favor of the spec-compliant behavior described above. This patch also
removes the `inline_block_overflow.html` reftest introduced in #3725, as
the behavior it expected contradicted CSS 2.1 (and in fact the test
fails in Gecko).

The changes that this patch makes to `input_selection_a.html` and
`input_selection_incremental_a.html` are necessary workarounds to make
the tests pass in light of the fact that Servo's UA stylesheet applies
`overflow: hidden` to `<input>` elements. I believe that the changes are
not necessary in other rendering engines because they hard-code
`overflow: hidden`-like behavior for `<input>` elements, while Servo
uses the actual CSS `overflow: hidden` behavior. As far as I can tell,
Servo's behavior is arguably more spec-compliant, but it remains to be
seen how Web compatible it is.

Improves the Google results pages.

Closes #13707.

r? @notriddle

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13732)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2016

💔 Test failed - linux-rel-css

@highfive
Copy link

highfive commented Oct 13, 2016

  ▶ PASS [expected FAIL] /css-flexbox-1_dev/html/flexbox_box-clear.htm
@notriddle
Copy link
Contributor

notriddle commented Oct 13, 2016

13.0909 < notriddle> crowbot: is flexbox_box-clear.htm intermittent?
13.0909 < crowbot> No intermittent issues filed with 'flexbox_box-clear.htm' in 
                   the title
fragments if `overflow` is not `visible` per CSS 2.1 § 10.8.1.

Additionally, this patch reverts the change introduced in #12642 in
favor of the spec-compliant behavior described above. This patch also
removes the `inline_block_overflow.html` reftest introduced in #3725, as
the behavior it expected contradicted CSS 2.1 (and in fact the test
fails in Gecko).

The changes that this patch makes to `input_selection_a.html` and
`input_selection_incremental_a.html` are necessary workarounds to make
the tests pass in light of the fact that Servo's UA stylesheet applies
`overflow: hidden` to `<input>` elements. I believe that the changes are
not necessary in other rendering engines because they hard-code
`overflow: hidden`-like behavior for `<input>` elements, while Servo
uses the actual CSS `overflow: hidden` behavior. As far as I can tell,
Servo's behavior is arguably more spec-compliant, but it remains to be
seen how Web compatible it is.

Improves the Google results pages.

Closes #13707.
@pcwalton pcwalton force-pushed the pcwalton:inline-block-vertical-align-overflow branch from 79de34b to c25dd2a Oct 13, 2016
@pcwalton
Copy link
Contributor Author

pcwalton commented Oct 13, 2016

@bors-servo: r=notriddle

@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2016

📌 Commit c25dd2a has been approved by notriddle

@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2016

Testing commit c25dd2a with merge bbcc743...

bors-servo added a commit that referenced this pull request Oct 13, 2016
… r=notriddle

layout: Use the margin box for vertical positioning of `inline-block` fragments if `overflow` is not `visible` per CSS 2.1 § 10.8.1.

Additionally, this patch reverts the change introduced in #12642 in
favor of the spec-compliant behavior described above. This patch also
removes the `inline_block_overflow.html` reftest introduced in #3725, as
the behavior it expected contradicted CSS 2.1 (and in fact the test
fails in Gecko).

The changes that this patch makes to `input_selection_a.html` and
`input_selection_incremental_a.html` are necessary workarounds to make
the tests pass in light of the fact that Servo's UA stylesheet applies
`overflow: hidden` to `<input>` elements. I believe that the changes are
not necessary in other rendering engines because they hard-code
`overflow: hidden`-like behavior for `<input>` elements, while Servo
uses the actual CSS `overflow: hidden` behavior. As far as I can tell,
Servo's behavior is arguably more spec-compliant, but it remains to be
seen how Web compatible it is.

Improves the Google results pages.

Closes #13707.

r? @notriddle

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13732)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2016

@bors-servo bors-servo merged commit c25dd2a into servo:master Oct 13, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.