Opened 8 months ago

Closed 2 months ago

#18880 closed bug (fixed)

HaikuWebKit (master) does not download

Reported by: vidrep Owned by: pulkomandy
Priority: normal Milestone: R1/beta5
Component: Applications/WebPositive Version: R1/beta4
Keywords: Cc:
Blocked By: Blocking: #18989
Platform: All

Description (last modified by vidrep)

hrev57691 x86_64

HaikuWebKit 1.9.11

WebKit 619.1.6

When downloading any file, the download progress bar shows progress, but when complete there is no file in the download folder.

Attachments (1)

screenshot1.png (438.3 KB ) - added by vidrep 8 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 by vidrep, 8 months ago

Description: modified (diff)

comment:2 by vidrep, 8 months ago

I have attached a screenshot showing how a downloaded PDF (or any file cannot be opened) after the download is complete. YES, I do have BePDF installed, in case anyone asks. Using wget does work fine.

by vidrep, 8 months ago

Attachment: screenshot1.png added

comment:3 by pulkomandy, 3 months ago

Milestone: UnscheduledR1/beta5

comment:4 by waddlesplash, 3 months ago

Blocking: 18989 added

comment:5 by pulkomandy, 3 months ago

A bit of context into why downloads are a bit problematic in WebKit currently.

We are using the cURL backend with WebKitLegacy. We are the only platform to do that (since the only other remaining WebKitLegacy platform is iOS, and they don't use cURL).

The network support for WebKitLegacy is quite different from WebKit 2. As a result, a lot of things were deleted from WebKit cURL implementation after it became unused by WebKitLegacy.

The relevant commits in WebKit are e28ed6a22681dd39c13bf7564fe9464b7f217241 and 3d96d2e0cc608ad4d77d7e5214ed2dc82838d02f. I have reverted both, but then there are occasional merge conflicts with updates, that I may not have correctly resolved.

Anyway, the way this works:

  • Start in WebKitLegacy in the BWebDownload class. This does not do much, but offload the work to WebDownloadPrivate (this is done for API isolation reasons: BWebDownload has a stable ABI and should not expose WebKit internal headers to the apps using the API).
  • WebDownloadPrivate has a lot of ifdefs for cURL, since the flow is quite different from libnetapi (the code supports both). For cURL, it calls into the CurlDownload class, which in turns enable a special "download" mode for CurlRequest, that writes the data to a file.
  • The cleanest option would be to rewrite WebDownloadPrivate in a way that it does not need CurlDownload and special support in CurlRequest. Then we can cancel all the changes there and have a "clean" way to do downloads. But I don't know if this is possible or how easy it is.
  • Otherwise, let's figure out why the code to do a download is not working. The request seems to be operating normally (as we see the download progress bar moving).
  • In fact, the file seems to be downloaded to a temporary file in /tmp. It seems CurlRequest was at some point supporting setting another path to download the file directly to another location, but this code is gone. Instead, files are downloaded to /tmp and then should be moved to their final download location only when the download is complete. This is not necessarily great, since we would like to be able to resume incomplete downloads. So that gives us another option: implement a CurlRequest::setDownloadFilePath method to set the file path before the request is started, then we can download in place. It looks like CurlRequest should already support this just fine, there just isn't any API to set the file path at the moment. Of course the path needs to be forwarded from CurlDownload to the CurlRequest.
  • Finally, we can see the file is created in /tmp but not moved. This agains seems to come from the fact that m_downloadFilePath is never set in CurlRequest. When it generates a temporary file name on its own, it should set this variable to point to it. So that CurlDownload can get that and use it to move the file. Currently, the variable remains empty, and so the code in CurlDownload::curlDidComplete ends up not moving the file.

In other words: CurlDownload expects that CurlRequest will write the file directly to the final destination, and CurlRequest writes to a temporary file and expects that CurlDownload will move the file when the download is complete.

comment:6 by pulkomandy, 3 months ago

Fixed in https://github.com/haiku/haikuwebkit/commit/8c749e6b6e705715d929aee16cfdb14fb186124c

Should I make a new HaikuWebKit release, or include the patch in haikuports?

comment:7 by waddlesplash, 3 months ago

I don't know if it makes much difference in the end, the builders will by now have to redownload the whole source archive.

comment:8 by pulkomandy, 3 months ago

I can use the opportunity to fix the version number I forgot to bump. I'll make a release tomorrow, then.

comment:9 by pulkomandy, 3 months ago

I released HaikuWebKit 1.9.13. So now we have to update the package mirror repo to include it.

comment:10 by pulkomandy, 3 months ago

Status: newin-progress

comment:11 by pulkomandy, 2 months ago

Resolution: fixed
Status: in-progressclosed

It should be working now.

Note: See TracTickets for help on using tickets.