Skip to content

Reorganise Save as... to Save article and allow saving as both pdf and mhtml#1434

Open
vighnesh-sawant wants to merge 1 commit intokiwix:mainfrom
vighnesh-sawant:context-menu-move-save-page-as-pdf
Open

Reorganise Save as... to Save article and allow saving as both pdf and mhtml#1434
vighnesh-sawant wants to merge 1 commit intokiwix:mainfrom
vighnesh-sawant:context-menu-move-save-page-as-pdf

Conversation

@vighnesh-sawant
Copy link
Copy Markdown
Contributor

@vighnesh-sawant vighnesh-sawant commented Dec 18, 2025

Fixes #1433

Most of the code change is adding the option to save as mhtml, should i split this off into another commit?
Also I have moved the downloadFinished() code out of the KProfile namespace to allow webview to also use it.

@vighnesh-sawant vighnesh-sawant force-pushed the context-menu-move-save-page-as-pdf branch 4 times, most recently from eb8696c to 1b11758 Compare December 18, 2025 07:13
@vighnesh-sawant vighnesh-sawant marked this pull request as ready for review December 18, 2025 07:24
@vighnesh-sawant
Copy link
Copy Markdown
Contributor Author

@kelson42 Please do have a look at this

@kelson42
Copy link
Copy Markdown
Collaborator

@vighnesh-sawant I just have tested and here my feedbacks:

  • Thank you for splitting this, in general it works, but we have to fix a few things

  • We should rephrase the dialog. I would propose title to be 'Article saved' with content 'The article "foobar" has been saved at: /my/path.pdf'. But maybe you have a better idea (would be great to have a toast message like on Android). 'ok' button should always be written uppercase (so 'OK').

image
  • If I save three time in a row the same article at the same path I get a warning tell me the file will be reaplaced (this part is OK)... but then the dialog box (above) is displayed 3 times in a row!

  • Don't call the mhtml format "Web archive" (which is pretty unclear for the end user), just says "Web Page, complete" (like on Firefox)

  • The file extension should not be visible in the filepicker dialog (so not foobar.pdf, but just foobar). This is important, otherwise you will have to handle the extension change, depending what people choose (which might not even be possible).

  • You have to add the file extension at save time. Use .pdf or .html (so no .mhtml). If you see that the user has already added an extension (so .pdf or .html) please don't add one on the top.

@vighnesh-sawant
Copy link
Copy Markdown
Contributor Author

vighnesh-sawant commented Jan 14, 2026

@kelson42
1.Will work on this.
Maybe we can make adding toasts a new issue? Qt does not have toasts by default, we would have to implement them/ import a library in our project for it.
2.Fixed this!
3. Done
4. Qt already handles the extension change, would you prefer no extension or qt handling it?
Also I wanted to confirm does it do that on your machine also? (I suspect it may not) (It does on mine)
5. Changed to html

@vighnesh-sawant vighnesh-sawant force-pushed the context-menu-move-save-page-as-pdf branch 3 times, most recently from 24c7cbe to d42dcc5 Compare January 14, 2026 12:25
@vighnesh-sawant
Copy link
Copy Markdown
Contributor Author

vighnesh-sawant commented Jan 14, 2026

1.Changed the dialog
image

@vighnesh-sawant
Copy link
Copy Markdown
Contributor Author

@kelson42 Bump!
I dont really know if the last comment pinged you or not.

@vighnesh-sawant vighnesh-sawant force-pushed the context-menu-move-save-page-as-pdf branch from d42dcc5 to ece28aa Compare February 15, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reorganise "Save as..." to save article

2 participants