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

Various textinput fixes #8983

Merged
merged 5 commits into from Jan 11, 2016
Merged

Various textinput fixes #8983

merged 5 commits into from Jan 11, 2016

Conversation

@Manishearth
Copy link
Member

Manishearth commented Dec 15, 2015

  • Currently the cursor sticks around if you click elsewhere. Now the text inputs are relayout-ed on blur.
  • Currently whitespace gets collapsed in text input (#8772). Not anymore.

Review on Reviewable

@highfive
Copy link

highfive commented Dec 15, 2015

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!
@Manishearth Manishearth changed the title Relayout text input elements on blur Various cursor fixes Dec 15, 2015
@Manishearth Manishearth changed the title Various cursor fixes Various textinput fixes Dec 15, 2015
@Manishearth
Copy link
Member Author

Manishearth commented Dec 15, 2015

r? @jdm

@frewsxcv
Copy link
Member

frewsxcv commented Dec 15, 2015

Want to add the regression test from #8722?

@Manishearth
Copy link
Member Author

Manishearth commented Dec 15, 2015

Done

@frewsxcv
Copy link
Member

frewsxcv commented Dec 15, 2015

Why not a WPT reftest?

@Manishearth
Copy link
Member Author

Manishearth commented Dec 15, 2015

Yeah, I'm working on that, after I get #8543 working too.

@Manishearth
Copy link
Member Author

Manishearth commented Dec 15, 2015

I added a reftest. It compares against things with   in their value (which used to work before). Let me know if I should add an image ref instead.

@KiChjang
Copy link
Member

KiChjang commented Dec 15, 2015

Travis CI fails to build right now.

$ bash etc/ci/manifest_changed.sh

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

index 84262fa..7fccb08 100644

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

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

@@ -2431,6 +2431,18 @@

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

           }

         ],

+        "css/input_whitespace_ref.html": [

+          {

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

+            "references": [

+              [

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

+                "=="

+              ]

+            ],

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

+          }

+        ],

         "css/inset.html": [

           {

             "path": "css/inset.html",

@@ -8316,6 +8328,18 @@

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

         }

       ],

+      "css/input_whitespace_ref.html": [

+        {

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

+          "references": [

+            [

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

+              "=="

+            ]

+          ],

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

+        }

+      ],

       "css/inset.html": [

         {

           "path": "css/inset.html",

The command "bash etc/ci/manifest_changed.sh" exited with 1.
@Manishearth
Copy link
Member Author

Manishearth commented Dec 16, 2015

I used create-wpt to create the test, and it added those changes. Not sure why they're failing.

@Manishearth Manishearth force-pushed the Manishearth:cursors branch from a4a4a18 to 98ce1c9 Dec 16, 2015
@Manishearth
Copy link
Member Author

Manishearth commented Dec 16, 2015

Fixed.

@bors-servo
Copy link
Contributor

bors-servo commented Dec 31, 2015

The latest upstream changes (presumably #9099) made this pull request unmergeable. Please resolve the merge conflicts.

@Manishearth Manishearth force-pushed the Manishearth:cursors branch from 98ce1c9 to f1c3494 Jan 2, 2016
@Manishearth
Copy link
Member Author

Manishearth commented Jan 2, 2016

Rebased.

@bors-servo
Copy link
Contributor

bors-servo commented Jan 3, 2016

The latest upstream changes (presumably #9072) made this pull request unmergeable. Please resolve the merge conflicts.

@Manishearth Manishearth force-pushed the Manishearth:cursors branch from f1c3494 to 539ee18 Jan 3, 2016
@Manishearth
Copy link
Member Author

Manishearth commented Jan 3, 2016

Rebased

@jdm jdm removed the S-awaiting-review label Jan 5, 2016
@Manishearth
Copy link
Member Author

Manishearth commented Jan 7, 2016

Done

@jdm
Copy link
Member

jdm commented Jan 7, 2016

@bors-servo: r+


Reviewed 1 of 1 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2016

📌 Commit f67e208 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2016

Testing commit f67e208 with merge 9d4cb85...

bors-servo added a commit that referenced this pull request Jan 7, 2016
Various textinput fixes

 - Currently the cursor sticks around if you click elsewhere. Now the text inputs are relayout-ed on blur.
 - Currently whitespace gets collapsed in text input (#8772). Not anymore.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8983)
<!-- Reviewable:end -->
@Manishearth
Copy link
Member Author

Manishearth commented Jan 11, 2016

@bors retry force clean

@Manishearth
Copy link
Member Author

Manishearth commented Jan 11, 2016

@bors r=jdm retry try- force clean

@KiChjang
Copy link
Member

KiChjang commented Jan 11, 2016

@bors-servo retry force clean

@Manishearth
Copy link
Member Author

Manishearth commented Jan 11, 2016

@bors-servo r=jdm retry force clean

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2016

📌 Commit f67e208 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2016

Testing commit f67e208 with merge 63251f1...

bors-servo added a commit that referenced this pull request Jan 11, 2016
Various textinput fixes

 - Currently the cursor sticks around if you click elsewhere. Now the text inputs are relayout-ed on blur.
 - Currently whitespace gets collapsed in text input (#8772). Not anymore.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8983)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2016

💔 Test failed - mac-rel-css

@Manishearth
Copy link
Member Author

Manishearth commented Jan 11, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2016

💣 Failed to start rebuilding: 404 - No Such Resource

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2016

Testing commit f67e208 with merge dbc2c2f...

bors-servo added a commit that referenced this pull request Jan 11, 2016
Various textinput fixes

 - Currently the cursor sticks around if you click elsewhere. Now the text inputs are relayout-ed on blur.
 - Currently whitespace gets collapsed in text input (#8772). Not anymore.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8983)
<!-- Reviewable:end -->
@Manishearth
Copy link
Member Author

Manishearth commented Jan 11, 2016

@bors retry clean

@Manishearth
Copy link
Member Author

Manishearth commented Jan 11, 2016

@bors-servo retry clean

@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2016

Testing commit f67e208 with merge 099beee...

bors-servo added a commit that referenced this pull request Jan 11, 2016
Various textinput fixes

 - Currently the cursor sticks around if you click elsewhere. Now the text inputs are relayout-ed on blur.
 - Currently whitespace gets collapsed in text input (#8772). Not anymore.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8983)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 11, 2016

@bors-servo bors-servo merged commit f67e208 into servo:master Jan 11, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Manishearth Manishearth deleted the Manishearth:cursors branch Jan 11, 2016
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

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