Fix unintended behaviors of Location.reload() - #781
Conversation
| final WebRequest request = htmlPage.getWebResponse().getWebRequest(); | ||
|
|
||
| // update request url with location.href in case hash was changed | ||
| request.setUrl(new URL(getHref())); |
There was a problem hiding this comment.
i fear setting this here will change the getUrl() response because the getUrl() method reads from the request
Can you please include the checking of the pageUrl in your test cases
There was a problem hiding this comment.
@rbri added 1 more commit to update the tests. Could you take a look at it ?
There was a problem hiding this comment.
do you see a chance to add the tests to Location2Test - then we can really compare with the browser behaviour
There was a problem hiding this comment.
What test cases do you want to put in Location2Test ? Same as this one ?
There was a problem hiding this comment.
What test cases do you want to put in Location2Test ? Same as this one ?
Sorry...
Usually it is easier to maintain the tests if they are WebDriverTest cases, because i can run them with real browsers and proof the expected results more or less automatically. Otherwise we have to recheck (in theory) manually with every browser update.
So what i like to say is to prefer writing WebDriverTest cases over the others. But in some cases it is not possible...
There was a problem hiding this comment.
I see... I will try to add it tomorrow
b35d2a8 to
94c8787
Compare
|
Have added two tests (validated with real browsers but failing in HtmlUnit). Maybe you can take this as sample.... Btw this shows that there is a difference between FF and Chrome |
033bbbe to
fadcfdf
Compare
|
@yamamoto-kanoko @duonglaiquang @atnak first of all thanks for all the work on this (and all the other stuff) and sorry for being that picky this time... When running the new Location2Test with real browsers (i guess you know how to setup this (see WebDriverTestCase class javadoc)) i got exactly the result i wrote into the expectations. If you think the test produces different results for the real browsers than doing the same with the real browser manually then we have to use the @BuggyWebDriver annotation. Suggestion - please have a look at the new tests in Location2Test
|
|
OK, have adjusted the tests, and now i can reproduce your results. |
|
@rbri Thanks for all the tests 😁. But regarding this one shouldn't FF & FF_ESR have null referer ?
|
fadcfdf to
4552c3d
Compare
|
And for this one, your previous test have
|
Yes; something strage is going on here with FF - just trying to write test that make clear what secret is hidden there. |
|
@yamamoto-kanoko @duonglaiquang @atnak have commited my updated tests and your fixes (without merging this). Looks like your fixes are fixing my tests - always a good sign :-) Thanks a lot and have a nice weekend.... |
|
@rbri @duonglaiquang @atnak |
|
made a first 4.2.0-SNAPSHOT available - have fun |
This PR does the following
Location.reload()behave like real browsers byreload()will not actually reload the page if the current href contains a hash, as showed inLocationTest.reloadWithHash()reload(), as showed inLocationTest.reloadNoHash()