Use floats to represent intended fixed position #11330

mitchhentges · 2016-05-22T18:21:19Z

Thank you for contributing to Servo! Please replace each [ ] by [X] when the step is complete, and replace __ with appropriate data:

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

Either:

There are tests for these changes

Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.

This change is

highfive · 2016-05-22T18:21:24Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @KiChjang (or someone else) soon.

highfive · 2016-05-22T18:32:57Z

New code was committed to pull request.

KiChjang · 2016-05-24T13:55:52Z

Fails the WPT lint:

diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json

index 1b3f596..04adcb1 100644

--- a/tests/wpt/mozilla/meta/MANIFEST.json

+++ b/tests/wpt/mozilla/meta/MANIFEST.json

@@ -4108,16 +4108,16 @@

             "url": "/_mozilla/css/position_abs_width_percentage_a.html"

           }

         ],

-        "css/position_fixed_a.html": [

+        "css/position_fixed.html": [

           {

-            "path": "css/position_fixed_a.html",

+            "path": "css/position_fixed.html",

             "references": [

               [

-                "/_mozilla/css/position_fixed_b.html",

+                "/_mozilla/css/position_fixed_ref.html",

                 "=="

               ]

             ],

-            "url": "/_mozilla/css/position_fixed_a.html"

+            "url": "/_mozilla/css/position_fixed.html"

           }

         ],

         "css/position_fixed_background_color_a.html": [

@@ -10920,16 +10920,16 @@

           "url": "/_mozilla/css/position_abs_width_percentage_a.html"

         }

       ],

-      "css/position_fixed_a.html": [

+      "css/position_fixed.html": [

         {

-          "path": "css/position_fixed_a.html",

+          "path": "css/position_fixed.html",

           "references": [

             [

-              "/_mozilla/css/position_fixed_b.html",

+              "/_mozilla/css/position_fixed_ref.html",

               "=="

             ]

           ],

-          "url": "/_mozilla/css/position_fixed_a.html"

+          "url": "/_mozilla/css/position_fixed.html"

         }

       ],

       "css/position_fixed_background_color_a.html": [

highfive · 2016-05-24T14:09:12Z

New code was committed to pull request.

KiChjang · 2016-05-24T14:17:12Z

tests/wpt/mozilla/tests/css/position_fixed_b.html

-      background: black;
-      top: 100px;
-      bottom: 30px;
+      float:left;


nit: Space after :.

KiChjang · 2016-05-24T14:18:34Z

r=me post-nit.

mitchhentges · 2016-05-24T15:19:41Z

Thanks for catching that, sorry for missing it

highfive · 2016-05-24T15:20:03Z

New code was committed to pull request.

KiChjang · 2016-05-24T15:25:11Z

@bors-servo r+

Thanks!

bors-servo · 2016-05-24T15:25:13Z

📌 Commit 01190cd has been approved by KiChjang

bors-servo · 2016-05-25T00:18:38Z

⌛ Testing commit 01190cd with merge a7d8bb1...


        Auto merge of #11330 - mitchhentges:1516-reference-test, r=KiChjang

Use floats to represent intended fixed position Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy --faster` does not report any errors - [X] These changes fix #1516 Either: - [X] There are tests for these changes Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11330)

bors-servo · 2016-05-25T00:53:03Z

💔 Test failed - mac-rel-css

cbrewster · 2016-05-25T00:55:25Z

@bors-servo retry

#11396

bors-servo · 2016-05-25T01:56:10Z

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, windows are reusable. Rebuilding only linux-rel, mac-rel-css, mac-rel-wpt...

bors-servo · 2016-05-25T02:35:34Z

💔 Test failed - mac-rel-wpt

highfive · 2016-05-25T02:35:36Z

  ▶ FAIL [expected PASS] /_mozilla/css/position_fixed_a.html
  └   → /_mozilla/css/position_fixed_a.html 998f40ff66ec57b74253e256fa27d9168f9eb3c5
/_mozilla/css/position_fixed_b.html 1a6c482c7e6e5582fd4dac44a054962d296e82d8
Testing 998f40ff66ec57b74253e256fa27d9168f9eb3c5 == 1a6c482c7e6e5582fd4dac44a054962d296e82d8

cbrewster · 2016-05-25T02:40:45Z

@bors-servo retry

#9106


        Auto merge of #11330 - mitchhentges:1516-reference-test, r=KiChjang

Use floats to represent intended fixed position Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy --faster` does not report any errors - [X] These changes fix #1516 Either: - [X] There are tests for these changes Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11330)

bors-servo · 2016-05-25T03:14:50Z

⌛ Testing commit 01190cd with merge ad2eb5c...

mbrubeck · 2016-05-25T03:16:55Z

@bors-servo r- force

This PR re-enables position_fixed_a.html so we shouldn't check it in if it's failing.

bors-servo · 2016-05-25T03:16:57Z

⛄ The build was interrupted to prioritize another pull request.

cbrewster · 2016-05-25T03:22:01Z

@mbrubeck ahh good catch, sorry about that!

mitchhentges · 2016-05-25T08:17:17Z

That is ... really weird. I ran it ~50 times locally, and it passed every time. Maybe there's something special with the Mac build, or build agent? Can I get any more information on the failure?

highfive · 2016-05-28T18:07:29Z

New code was committed to pull request.

emilio · 2016-05-28T18:07:55Z

@bors-servo: try

bors-servo · 2016-05-28T18:07:57Z

⌛ Trying commit 319bb93 with merge 7313c38...


        Auto merge of #11330 - mitchhentges:1516-reference-test, r=<try>

Use floats to represent intended fixed position Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy --faster` does not report any errors - [X] These changes fix #1516 Either: - [X] There are tests for these changes Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11330)

emilio · 2016-05-28T18:08:48Z

log

@@ -0,0 +1,95 @@
+{"source": "web-platform-tests", "thread": "MainThread", "time": 1464458627264, "action": "log", "message": "Using 6 client processes", "level": "INFO", "pid": 3625}


This file would need to go before landing this btw

mitchhentges · 2016-05-28T18:09:32Z

So, I've updated the tests to assume that the font will be rendered as black squres. Though two html files won't match up with ./mach run (because 'Ahem' is actually rendered), it will be rendered consistently as black squares on all machines.

It's OK that the font is a black screen, because this test checks the positioning of fixed and dynamically-sized containers. Since even black-square-rendered text is still dynamically sized, the test is still valid

highfive · 2016-05-28T18:10:11Z

New code was committed to pull request.

mitchhentges · 2016-05-28T18:14:18Z

That would explain a lot of things. Maybe ./mach run is the thing that doesn't include the font?

I'll update the comment if we can confirm that Ahem is square-y. I thought it was a bug on my end that gnome-font-viewer was seeing squares

emilio · 2016-05-28T18:15:54Z

Whoops, sorry, deleted my previous comment thinking I had misread you but...

Yes, Ahem is AFAIK square-y, that's the point and why it's used for testing :-)

emilio · 2016-05-28T18:16:21Z

I think @mbrubeck can confirm it if you need more confirmation

jdm · 2016-05-29T03:55:23Z

Yes, Ahem doesn't contain actual letters, only solid squares.


        use floats to represent intended fixed position, re-enable position_f…

…ixed test

highfive · 2016-05-29T10:02:58Z

New code was committed to pull request.

mitchhentges · 2016-05-29T10:03:27Z

I think that it's totally good now

KiChjang · 2016-05-31T06:27:51Z

@bors-servo try


        Auto merge of #11330 - mitchhentges:1516-reference-test, r=<try>

Use floats to represent intended fixed position Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy --faster` does not report any errors - [X] These changes fix #1516 Either: - [X] There are tests for these changes Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11330)

bors-servo · 2016-05-31T06:27:54Z

⌛ Trying commit 94a884b with merge 05165b8...

bors-servo · 2016-05-31T07:16:01Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

KiChjang · 2016-05-31T14:51:08Z

@bors-servo r+

bors-servo · 2016-05-31T14:51:09Z

📌 Commit 94a884b has been approved by KiChjang

bors-servo · 2016-05-31T14:51:12Z

⌛ Testing commit 94a884b with merge 55b0bb0...


        Auto merge of #11330 - mitchhentges:1516-reference-test, r=KiChjang

Use floats to represent intended fixed position Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy --faster` does not report any errors - [X] These changes fix #1516 Either: - [X] There are tests for these changes Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11330)

bors-servo · 2016-05-31T15:58:01Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

highfive assigned KiChjang May 22, 2016

highfive added the S-awaiting-review label May 22, 2016

mitchhentges force-pushed the mitchhentges:1516-reference-test branch from 59b72af to 849e67a May 22, 2016

mitchhentges force-pushed the mitchhentges:1516-reference-test branch from 849e67a to 3186046 May 24, 2016

KiChjang reviewed May 24, 2016
View changes

tests/wpt/mozilla/tests/css/position_fixed_b.html Outdated

background: black;

top: 100px;

bottom: 30px;

float:left;

This comment has been minimized.

Sign in to view

Copy link

KiChjang May 24, 2016
Member

nit: Space after :.

mitchhentges force-pushed the mitchhentges:1516-reference-test branch from 3186046 to 01190cd May 24, 2016

mitchhentges force-pushed the mitchhentges:1516-reference-test branch from 01190cd to de75884 May 25, 2016

mitchhentges force-pushed the mitchhentges:1516-reference-test branch from 95d7500 to 319bb93 May 28, 2016

emilio reviewed May 28, 2016
View changes

mitchhentges force-pushed the mitchhentges:1516-reference-test branch from 319bb93 to 51eed25 May 28, 2016

use floats to represent intended fixed position, re-enable position_f…

Loading status checks…

94a884b

…ixed test

mitchhentges force-pushed the mitchhentges:1516-reference-test branch from 51eed25 to 94a884b May 29, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels May 31, 2016

bors-servo merged commit 94a884b into servo:master May 31, 2016
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

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

homu Test successful
Details

highfive removed the S-awaiting-merge label May 31, 2016

		@@ -0,0 +1,95 @@
		{"source": "web-platform-tests", "thread": "MainThread", "time": 1464458627264, "action": "log", "message": "Using 6 client processes", "level": "INFO", "pid": 3625}

servo / servo

Join GitHub today

Use floats to represent intended fixed position #11330

Use floats to represent intended fixed position #11330

Conversation

mitchhentges commented May 22, 2016 • edited by larsbergstrom

highfive commented May 22, 2016

highfive commented May 22, 2016

KiChjang commented May 24, 2016

highfive commented May 24, 2016

This comment has been minimized.

KiChjang May 24, 2016

KiChjang commented May 24, 2016

mitchhentges commented May 24, 2016

highfive commented May 24, 2016

KiChjang commented May 24, 2016

bors-servo commented May 24, 2016

bors-servo commented May 25, 2016

bors-servo commented May 25, 2016

cbrewster commented May 25, 2016

bors-servo commented May 25, 2016

bors-servo commented May 25, 2016

highfive commented May 25, 2016

cbrewster commented May 25, 2016

bors-servo commented May 25, 2016

mbrubeck commented May 25, 2016

bors-servo commented May 25, 2016

cbrewster commented May 25, 2016

mitchhentges commented May 25, 2016

highfive commented May 28, 2016

emilio commented May 28, 2016

bors-servo commented May 28, 2016

This comment has been minimized.

emilio May 28, 2016

mitchhentges commented May 28, 2016

highfive commented May 28, 2016

mitchhentges commented May 28, 2016

emilio commented May 28, 2016

emilio commented May 28, 2016

jdm commented May 29, 2016

highfive commented May 29, 2016

mitchhentges commented May 29, 2016

KiChjang commented May 31, 2016

bors-servo commented May 31, 2016

bors-servo commented May 31, 2016

KiChjang commented May 31, 2016

bors-servo commented May 31, 2016

bors-servo commented May 31, 2016

bors-servo commented May 31, 2016

mitchhentges commented May 22, 2016 •

edited by larsbergstrom