Opened 2 years ago

Last modified 16 months ago

#13550 new bug

WebPositive history is not updated in certain cases

Reported by: accessays Owned by: pulkomandy
Priority: normal Milestone: Unscheduled
Component: Applications/WebPositive Version: R1/Development
Keywords: url, webpositive, history, navigation Cc: accessays
Blocked By: Blocking:
Has a Patch: Platform: x86

Description

  1. Open WebPositive
  2. Navigate to https://github.com/haiku/haiku
  3. Click on any folder (e. g. 3rdparty, src, etc.)
  4. Observe the URL input and navigation buttons not updating

Tested with hrev52001 and hrev52003, gcc2 hybrid.

It seems like Load... functions never get called in BrowserWindow.

Attachments (2)

0001-WebPositive-fixed-a-lot-about-how-navigation-works-m.patch (10.0 KB ) - added by accessays 2 years ago.
Patch #1
0001-Fixed-again-the-way-URL-locking-works.patch (8.0 KB ) - added by accessays 2 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 by accessays, 2 years ago

I will attempt to solve this myself.

comment:2 by accessays, 2 years ago

Has a Patch: set

comment:3 by accessays, 2 years ago

Here is a patch that seems to fix that issue, as well as several other issues. I suggest trying to browse the web, especially GitHub, and some websites that do redirects (or just open a simple HTML file with JS redirect in it) with current Web+ and the patched version to really see the difference.

comment:4 by pulkomandy, 2 years ago

The patch seems to remove your previous changes with regard to locking the URL bar and timeout on said locking. Is that intentional?

I'll need to review the other changes more carefully (probably this evening). I thought the WebKit engine would do most of the work on this, but I could be wrong.

in reply to:  4 comment:5 by accessays, 2 years ago

Replying to pulkomandy:

The patch seems to remove your previous changes with regard to locking the URL bar and timeout on said locking. Is that intentional?

Yes, it is. This patch changes the way locking works - instead of a timeout it relies on checking whether the URL was actually changed by the user or not.

I thought the WebKit engine would do most of the work on this, but I could be wrong.

It also felt like it should be, but the original version sometimes messed up the navigation controls (i. e. go to YouTube, then click the YouTube logo again, and the stop icon will not become disabled)

comment:6 by pulkomandy, 2 years ago

Ok, I took a closer look at the patch. I think it goes the wrong way about this.

The idea is that WebKit should be handling all this stuff, and telling us what to do: should the stop button be enabled? Is there a new entry in the history, or is the load just a redirect of a previous request? etc.

The application should not get to handle the LOAD_DL_COMPLETED, LOAD_COMMITTED, etc messages by itself (it can do so, but only to handle other specific things - not the button states that are handled by WebKit). Likewise, calling ResendNotifications directly is not the right thing to do - this should be done on WebKit side.

The reason which makes this important is that WebPositive isn't the only browser using the API. In fact, the WebKit test suite is run with a smaller browser (called HaikuLauncher), and this should be kept behaving the same as the real Web+. Otherwise, the testsuite is not very useful.

So, your changes may be correct, but they should be implemented on WebKit side, at Source/WebKit/haiku/API/BrowserWindow.cpp . This way, everyone using WebKit will get a working stop button, not just Web+.

With regard to the changes about the URL locking, they look good, and I could apply just that part if it is split out of the patch file.

in reply to:  6 comment:7 by accessays, 2 years ago

Replying to pulkomandy:

So, your changes may be correct, but they should be implemented on WebKit side, at Source/WebKit/haiku/API/BrowserWindow.cpp . This way, everyone using WebKit will get a working stop button, not just Web+.

I'll take a look into it.

With regard to the changes about the URL locking, they look good, and I could apply just that part if it is split out of the patch file.

Sure, new patch file provided.

comment:8 by pulkomandy, 16 months ago

Has a Patch: unset

comment:9 by pulkomandy, 16 months ago

Has a Patch: unset

Marking remaining patch as obsolete, the changes have been made on WebKit side.

Note: See TracTickets for help on using tickets.