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

Fixed bug with IE and basic auth #369

Merged
merged 5 commits into from Aug 9, 2016

Conversation

Projects
None yet
3 participants
@simple-elf
Contributor

simple-elf commented Aug 9, 2016

Problem mentioned in #366 and highlighted in #320.
So here is quick fix of that. I think if there is empty login parameter we don't need to handling alert for basic auth.

@anilreddy

This comment has been minimized.

Show comment
Hide comment
@anilreddy

anilreddy Aug 9, 2016

Contributor

@simple-elf Thanks for the quick fix :-)

Contributor

anilreddy commented Aug 9, 2016

@simple-elf Thanks for the quick fix :-)

@simple-elf

This comment has been minimized.

Show comment
Hide comment
@simple-elf

simple-elf Aug 9, 2016

Contributor

Sorry for commit spam, I'm new on github =)

Contributor

simple-elf commented Aug 9, 2016

Sorry for commit spam, I'm new on github =)

@anilreddy

This comment has been minimized.

Show comment
Hide comment
@anilreddy

anilreddy Aug 9, 2016

Contributor

@simple-elf also can you add a test-case for it.

Contributor

anilreddy commented Aug 9, 2016

@simple-elf also can you add a test-case for it.

@asolntsev asolntsev self-assigned this Aug 9, 2016

@asolntsev asolntsev added this to the 3.9 milestone Aug 9, 2016

@asolntsev asolntsev merged commit 4a052d2 into codeborne:master Aug 9, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Aug 9, 2016

Member

@simple-elf Thank you for the fix.

NB! Why do you think that this is "quick fix" Do you think that it's a "dirty temporary solution"? Do you have an idea of f better solution?

Member

asolntsev commented Aug 9, 2016

@simple-elf Thank you for the fix.

NB! Why do you think that this is "quick fix" Do you think that it's a "dirty temporary solution"? Do you have an idea of f better solution?

asolntsev added a commit that referenced this pull request Aug 9, 2016

@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Aug 9, 2016

Member

@simple-elf @anilreddy I have merged the fix and released selenide 3.8.1
It will appear in maven central repo in few hours.

Member

asolntsev commented Aug 9, 2016

@simple-elf @anilreddy I have merged the fix and released selenide 3.8.1
It will appear in maven central repo in few hours.

@simple-elf

This comment has been minimized.

Show comment
Hide comment
@simple-elf

simple-elf Aug 10, 2016

Contributor

We need to test not only this fix, but all code in IE too, but it's hard to find such service for IE. Maybe SauceLabs? I need to investigate it.

Contributor

simple-elf commented Aug 10, 2016

We need to test not only this fix, but all code in IE too, but it's hard to find such service for IE. Maybe SauceLabs? I need to investigate it.

@simple-elf

This comment has been minimized.

Show comment
Hide comment
@simple-elf

simple-elf Aug 10, 2016

Contributor

Test shoul be smth like this

  @Test
  public void canPassWithoutBasicAuthInIe() {
    assumeTrue(isIE());
    Selenide.open("http://httpbin.org/get");
    assertThat(source(), containsString("\"url\": \"http://httpbin.org/get\"")); // "": ""
  }
Contributor

simple-elf commented Aug 10, 2016

Test shoul be smth like this

  @Test
  public void canPassWithoutBasicAuthInIe() {
    assumeTrue(isIE());
    Selenide.open("http://httpbin.org/get");
    assertThat(source(), containsString("\"url\": \"http://httpbin.org/get\"")); // "": ""
  }
@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Aug 10, 2016

Member

+1
Actually we need a VPS with Windows. Because we need to

  1. run Selenide tests in IE, and
  2. build NSelenide - selenide port to .NET

Don't you know a good and cheap Windows hosting?

Member

asolntsev commented Aug 10, 2016

+1
Actually we need a VPS with Windows. Because we need to

  1. run Selenide tests in IE, and
  2. build NSelenide - selenide port to .NET

Don't you know a good and cheap Windows hosting?

@simple-elf

This comment has been minimized.

Show comment
Hide comment
@simple-elf

simple-elf Aug 10, 2016

Contributor

For running tests in IE we probably can use SauceLabs, it has free VM's for open source projects with IE

Contributor

simple-elf commented Aug 10, 2016

For running tests in IE we probably can use SauceLabs, it has free VM's for open source projects with IE

@simple-elf

This comment has been minimized.

Show comment
Hide comment
@anilreddy

This comment has been minimized.

Show comment
Hide comment
@anilreddy

anilreddy Aug 11, 2016

Contributor

@simple-elf yeah am already using it.

Contributor

anilreddy commented Aug 11, 2016

@simple-elf yeah am already using it.

BorisOsipov added a commit to BorisOsipov/selenide that referenced this pull request Nov 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment