kwebio / kweb-core

A Kotlin web framework
https://docs.kweb.io/book
GNU Lesser General Public License v3.0
969 stars 57 forks source link

Fix possible Firefox routing bug #50

Closed sanity closed 5 years ago

sanity commented 5 years ago

reported by: @MrPowerGamerBR

Also, I think I found a bug (only happens in Firefox)

If I'm accessing the website (example: 127.0.0.1/path), if I change the URL on the address bar and change it to 127.0.0.1/anotherpath, it doesn't redirect it stays on the 127.0.0.1/path it works on chromium.

alkoclick commented 5 years ago

I am also experiencing this, while using Firefox Quantum 67.0 on Ubuntu

sanity commented 5 years ago

Will investigate.

alkoclick commented 5 years ago

The following fails for me on my home desktop, also running Firefox Quantum 67.0 on Ubuntu but a different version (18.04)

By fails I mean: -> The link preview displays properly when I hover -> If I ctrl-click it, the new tab opens in the correct url -> If I normally click it, the page reloads but nothing happens, the url doesn't change -> If I change the url manually in this page, it reloads and remains in the same place (even if I would navigate to a valid url in the page) -> Shit I just realized that it also stops me if I try to go to another page, like google.com. My browser tab is literally imprisoned in this url!

fun main() {
    Kweb(port = 16097) {
        doc.body.new {
            route {
                path("/aPath") { 
                    h1().text("The page changed")
                }
                path("/") {
                    a(href = "/aPath").text("Click me!")
                }
            }
        }
    }
}

As noted above, this works as expected in chromium

ethanmdavidson commented 5 years ago

From the firefox console, when you click the link:

Navigated to http://localhost:16097/aPath
Socket closed localhost:16097:138:29
WebSocket was closed without a reason specified 
close { target: WebSocket, isTrusted: true, wasClean: true, code: 1001, reason: "", srcElement: WebSocket, currentTarget: WebSocket, eventPhase: 2, bubbles: false, cancelable: false, … }
localhost:16097:146:29
Forcing page reload localhost:16097:148:29
Navigated to http://localhost:16097/

It tries to navigate, which closes the websocket, firing the onClose event which reloads the current page, overriding the navigation.

Apparently, the proper way to handle this is to attempt to reconnect if CloseEvent.wasClean is true. The reason it works on chrome is because chrome skips the onClose event and goes straight to loading the page. See this stackoverflow answer

Should be fixed in b98ccc204b73804f90f6f4b3c45020c09731de37, please see if it works for you.

I'll write a test to cover this and figure out how to have selenium run the tests with firefox as well as chrome.

alkoclick commented 5 years ago

How can I test if it is fixed? Please don't tell me to build from source and include in my project :P Is the new version on JitPack?

ethanmdavidson commented 5 years ago

Yeah, it looks like JitPack allows you to use the commit hash as a dependency: https://jitpack.io/#kwebio/core/b98ccc204b

try changing your kweb dependency to com.github.kwebio:core:b98ccc204b. You might also be able to get the latest from the master branch by using com.github.kwebio:core:master-SNAPSHOT

In other words, the answer is to build from source and include in your project 😈 but jitpack seems to make it easy. I've never tried this before so let me know if it works.

alkoclick commented 5 years ago

I'm using gradle for dependency management and build automation. It actually managed to pull it using

dependencies {
    compile 'com.github.kwebio:core:b98ccc204b'
}

but then I'm getting a different problem (which may need its own issue probably) -> This turned out to be somewhere in my code, despite the completely unhelpful kotlin trace

alkoclick commented 5 years ago

Alright, can confirm fix works as expected! :)

sanity commented 5 years ago

Rolling out 0.4.19 in a minute which will include this fix. Thanks guys.