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

Rack-test driver adds additional carriage return to CR+LF in textareas #2547

Merged

Conversation

hoffi
Copy link
Contributor

@hoffi hoffi commented May 6, 2022

According to the HTML spec line breaks in forms should be represented as CR+LF (\r\n). The rack-test driver correctly converts this when a textarea contains \n. However if the textarea already had \r\n the driver adds another \r after the form is submitted resulting in an additional carriage return: \r\r\n.

This change does not add an additional carriage return when there already is one.

@twalpole
Copy link
Member

twalpole commented May 7, 2022

Unfortunately this test fails with a real browser (Chrome). You can run the chrome tests via HEADLESS=true WEBDRIVERS=true bundle exec rake spec_chrome. and if you want to just run one test tag it with :focus_ metadata

@hoffi
Copy link
Contributor Author

hoffi commented May 13, 2022

The test runs successful for me. Does it fail with a specific chrome version?

I have added :focus_ to my new test:

diff --git a/lib/capybara/spec/session/fill_in_spec.rb b/lib/capybara/spec/session/fill_in_spec.rb
index d56074b6..eb01eb99 100644
--- a/lib/capybara/spec/session/fill_in_spec.rb
+++ b/lib/capybara/spec/session/fill_in_spec.rb
@@ -98,7 +98,7 @@ Capybara::SpecHelper.spec '#fill_in' do
     expect(extract_results(@session)['description']).to eq("\r\nSome text\r\n")
   end
 
-  it 'should handle carriage returns with line feeds in a textarea correct' do
+  it 'should handle carriage returns with line feeds in a textarea correct', :focus_ do
     @session.fill_in('form_description', with: "\r\nSome text\r\n")
     @session.click_button('awesome')
     expect(extract_results(@session)['description']).to eq("\r\nSome text\r\n")

And ran the chrome test:

$ google-chrome-stable --version
Google Chrome 101.0.4951.64
$ HEADLESS=true WEBDRIVERS=true bundle exec rake spec_chrome
ruby -I./capybara/vendor/bundle/ruby/3.0.0/gems/rspec-core-3.11.0/lib:./capybara/vendor/bundle/ruby/3.0.0/gems/rspec-support-3.11.0/lib ./capybara/vendor/bundle/ruby/3.0.0/gems/rspec-core-3.11.0/exe/rspec --pattern ./spec/\{selenium_spec_chrome.rb\} --color
Capybara starting Puma...
* Version 5.6.4 , codename: Birdie's Version
* Min threads: 0, max threads: 4
* Listening on http://127.0.0.1:45121
Run options:
  include {:focus_=>true}
  exclude {:requires=>#<Proc:>}

Randomized with seed 60263
.2022-05-13 13:07:22 WARN SeleniumStatistics Executed a total of 22 commands in 0.3 seconds

execute_script:            executions:        8; total seconds: 0.0; avg sec/cmd: 0.003; total:  2.27%
find_elements:             executions:        4; total seconds: 0.0; avg sec/cmd: 0.009; total:  3.25%
get_window_handles:        executions:        2; total seconds: 0.0; avg sec/cmd: 0.002; total:  0.28%
get:                       executions:        2; total seconds: 0.1; avg sec/cmd:  0.04; total:   7.0%
send_command:              executions:        2; total seconds: 0.0; avg sec/cmd: 0.002; total:   0.4%
switch_to_window:          executions:        1; total seconds: 0.0; avg sec/cmd: 0.003; total:  0.25%
element_click:             executions:        1; total seconds: 0.0; avg sec/cmd: 0.035; total:  3.12%
element_send_keys:         executions:        1; total seconds: 0.0; avg sec/cmd: 0.023; total:   2.0%
new_session:               executions:        1; total seconds: 0.1; avg sec/cmd: 0.089; total:  7.83%



Finished in 0.22887 seconds (files took 1.1 seconds to load)
1 example, 0 failures

Randomized with seed 60263

And also with Chrome Beta:

$ google-chrome-beta --version
Google Chrome 102.0.5005.49 beta
$ HEADLESS=true WEBDRIVERS=true CHROME_BETA=true bundle exec rake spec_chrome
ruby -I./capybara/vendor/bundle/ruby/3.0.0/gems/rspec-core-3.11.0/lib:./capybara/vendor/bundle/ruby/3.0.0/gems/rspec-support-3.11.0/lib ./capybara/vendor/bundle/ruby/3.0.0/gems/rspec-core-3.11.0/exe/rspec --pattern ./spec/\{selenium_spec_chrome.rb\} --color
Capybara starting Puma...
* Version 5.6.4 , codename: Birdie's Version
* Min threads: 0, max threads: 4
* Listening on http://127.0.0.1:42341
Run options:
  include {:focus_=>true}
  exclude {:requires=>#<Proc:>}

Randomized with seed 64440
.2022-05-13 13:12:02 WARN SeleniumStatistics Executed a total of 22 commands in 0.3 seconds

execute_script:            executions:        8; total seconds: 0.0; avg sec/cmd: 0.003; total:  2.22%
find_elements:             executions:        4; total seconds: 0.0; avg sec/cmd: 0.009; total:  3.15%
get_window_handles:        executions:        2; total seconds: 0.0; avg sec/cmd: 0.001; total:  0.25%
get:                       executions:        2; total seconds: 0.1; avg sec/cmd: 0.041; total:   7.2%
send_command:              executions:        2; total seconds: 0.0; avg sec/cmd: 0.002; total:  0.38%
switch_to_window:          executions:        1; total seconds: 0.0; avg sec/cmd: 0.003; total:  0.23%
element_click:             executions:        1; total seconds: 0.0; avg sec/cmd: 0.041; total:  3.64%
element_send_keys:         executions:        1; total seconds: 0.0; avg sec/cmd: 0.021; total:  1.84%
new_session:               executions:        1; total seconds: 0.1; avg sec/cmd: 0.089; total:  7.82%



Finished in 0.22655 seconds (files took 1.1 seconds to load)
1 example, 0 failures

Randomized with seed 64440

When i run the whole test suite (without the focus metadata) it also finishes successfully.

@hoffi hoffi force-pushed the fix-carriage-returns-in-rack-test branch from 3265b3c to 9122dd1 Compare May 13, 2022 11:37
@twalpole
Copy link
Member

@hoffi Interesting -- it was failing for me, but is no longer - possible there was an issue with an earlier version of chromedriver/chrome .... thanks for testing it again

@twalpole twalpole merged commit 270f7b4 into teamcapybara:master May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants