What would you think of using a different name than http://apps.kde.org.uk/ that clearly shows it's non official or a dev thing?

ok :)

Does it make sense to use MEDIA_REWRITABLE_DVD there?

Depends, KUrl? Yes Kurl is deprecated and you should use QUrl

at this point i'd say that krazy warning is a bit old fashioned, but sure, won't hurt either

In D21734#478376, @davidhurka wrote:

Part calls them like this:

• setupActions() If not in printPreview mode nor in viewerWidget mode
• setupViewerActions() If not in printPreview mode.
• setupBaseActions() Always.
  
  Thinking how to describe that...

i got bored of reviewing before reaching 25%, sorry

In D21734#477729, @davidhurka wrote:

Oh, does someone know any intended purpose division between setupActions(), setupViewerActions(), and setupBaseActions()?

I'm pretty sure there's people there still using KDE4 (real life scenario: my father) (almost real life scenario: i know the Limux people were planning a migration from kde4 to Plasma 5, not sure if it already happened or not though).

Loooks good.

Honestly i'd prefer if we wait for 5.10 and then use QMetaObject::invokeMethod instead of QTimer::singleShot.

You're the only one that has made commits since 2016, so i'd say just commit?

I don't know if it's what you where trying to fix or not, but when running with

Please add the two suggested const and commit :)

In D14527#467671, @ngraham wrote:

The diff is so trivial, I say just submit a new one yourself with your own authorship information, if that's the sticking point.

Yes i know you've mentioned it earlier.

Code makes sense, haven't tried it though.

this doesn't get you anywhere though, at least you still have themes and the knsrc file that are needed from make install, no?

What's the problem with make install? it's literally 1 line

Personally i don't see how this helps at all, but other apps are doing it, so if you're sure it really works you can count my vote as -0

You have a memory leak and some other things like the warnings, but i'll fix them myself and commit.

A little of both?

@ngraham what is being requested is using existing code that does this instead of reinveinting the wheel.

Ahhhh, yes, the everyone does it wrong except me reasoning.

But we're setting the menu here no?

In D21630#475517, @ndavis wrote:

In D21630#475501, @aacid wrote:

wow Not even one minute to review things

I don't particularly like this, input-mouse is an icon that exists in most icon themes
extra/adwaita-icon-theme
extra/breeze-icons
extra/efl
extra/gnome-icon-theme
extra/gnome-icon-theme-symbolic
extra/gnome-themes-extra
extra/oxygen-icons
extra/oxygen-icons-svg
extra/qt5-doc
extra/xf86-input-mouse
extra/xfce4-power-manager
community/arc-icon-theme
community/elementary-icon-theme
community/faba-icon-theme
community/faenza-icon-theme
community/lxinput
community/lxinput-gtk3
community/mate-icon-theme
community/mate-icon-theme-faenza
community/mate-themes
community/moka-icon-theme
community/papirus-icon-theme

transform-browse exists only in breeze-icons

but 🤷‍♂️ i guess

Sorry, I'll wait longer in the future, but input-mouse is still wrong. This simply doesn't mean the same thing as "grabber" or "browse": https://raw.githubusercontent.com/GNOME/adwaita-icon-theme/master/Adwaita/22x22/legacy/input-mouse.png

I don't know why you say "it's a more common way", kmail does exactly what okular did, thus, being consistent.

wow Not even one minute to review things

*Do not change the shortcuts*

The diff shown here is wrong. We want the diff from your branch against master, not the last change in your branch against itself

Some comments.

You really need to remove all the mentions of page you're adding, this is geometry that has nothing to do with pages.

just commit and try it

In D21542#473116, @elvisangelaccio wrote:

Note: as side-effect of this patch, now binaries/libraries in an archive will open in Okular, because there is a okularapplication_fb desktop file registered with application/x-sharedlib.

@aacid why is okular associated with that mimetype?

In T10159#186753, @jriddell wrote:

In T10159#186751, @aacid wrote:

Do you think "Page Setup Info" should be there? Feels weird to me, we don't really have that in the rests of the web.

Yeah it's a complex site and there will be 100 people with an interest in keeping it up to date so I think an obvious link on how to do that is useful. I've moved it to float: right now so it's more out the way.

right, see https://cgit.kde.org/ktuberling.git/commit/?id=10d753fce6718a147f0095d2274515bf96078292 for inspiration

Nice work in general, some little things i found.

Actually no, this is not all good.

ther'es some small things as not compating against "" but using isEmpty, but i'll fix that myself when commititng

I haven't tested it and haven't tried understanding the code farther than "this is a bunch of painting code".

Honestly i don't think documenting things at this low level makes sense, it'll break because people will never remember to update comments even if it's just on the line above the code they are changing.