Note: This is not a working version, see my inline comment. However, I can confirm that after fixing the seg fault error, it will become working since I have manually tested to select the entire page and then qDebug() the "text" gave same contents as that of the selected page.
Interesting feature. Can confirm that it works for me; tested with a bunch of PDFs and ePubs with selectable text. No effect with documents that don't have selectable text. No crashes.
Seems sane enough to me. However I wonder if maybe triple-click might be better though? Triple-click is used for "select everything" in word processors, so perhaps we could take advantage of some familiarity there?
In the future for user interface or user-facing changes, please add VDG instead of me directly (that way other UI and UX designers can see it too)
A single click to set the cursor, a double click to the select the current word under the cursor and a triple click to select a whole line is the standard behaviour in most applications I know.
Currently you can select everything by CTRL+A. So, maby for a whole single page, it could be CTRL+SHIFT+A? Or ALT+Triple Click?
- Add a new action "Select Page" similar to "Select All"
- Call first that new action/function, then the copy function, instead of coding explicit in mouse event
- Shorten your comments, don't say what is obviously, explain things which may indistinct e.g."select the word which was clicked on" "select the entire page if click event is in empty portion"
I would prefer triple-click too, or a keyboard shortcut, as alexde suggested.
Selecting the whole page makes a big part of the screen flash, which is annoying when you accidentally do a double-click. This often happens with touchpads, when you want to drag something, but lose contact after the second tap. And to select a line with Text Selection Tool, you will need this double-click-drag.
Please test your changes. The new menu is not actually added to the Edit menu and the shortcut you chose conflicts with the print shortcut.
This icon is already used for the Select All action, and we don't want to re-use the same icon for two actions in the same menu. Let's remove it.
The correct text here would be "Select all text on current page"
Erm, that's the shortcut used for printing. Did you test this?
I knew ctrl p is for print, so for time being I kept it so I can get suggestion for other shortcut sequence. Also, I can't tell why entry is not added into Edit menu even though I had added that in docbook.
This patch works with ctrl p selecting the text(I checked after changing default shortcut for Print), only thing being absence of the corresponding entry in Edit.
It's not acceptable to deliberately publish a diff that does the wrong thing purely for the purpose of soliciting comments without adding [WIP] or [RFC] to the title. What if nobody noticed this and it landed as-is and broke printing?
Also, I can't tell why entry is not added into Edit menu even though I had added that in docbook.
The docbook is just for documentation. To put it in the menu structure, you need to add the action to part.rc and bump the version number at the top of the file.
The test plan still says that Ctrl+P will select the page ("or press combination of CTRL and P to select the entire page"). As far as I understand the previous comments, this is Alt+P now, isn't it? (I didn't do any tests.) Can you update the description accordingly?
To be honest, I just don't feel great about this. The original bug (select whole page's text on double-click) was a bad idea, and we've tried to torture this patch into implementing the requested feature in a sort of awkward way, with a top-level menu item in the Edit menu that has no icon and probably no keyboard shortcut--all for a function that's likely to be used infrequently--if ever--for most users.
I think I would feel better if we instead did the following:
- Triple-click to select current line or current paragraph
- Quadruple-click to select whole page
What do other folks think?
Because Alt is used for Access Keys (Mnemonics), right?
I thought, this should get quadruple-click and Ctrl+Shift+A.
Text Selection mode already pollutes the clipboard, so this action could also be called “Copy all text on current page”. And then, it would also make sense in the other modes, like Select All.
Considering the clipboard and how long Okular hangs to “select” a catalogue or datasheet with 1000+ pages, this action should get Ctrl+A. The other action would become “Select all Text on all Pages”, with Ctrl+Shift+A. This makes it less easy to hang Okular unintentionally.
And, how often would someone want to select the whole document? The result would include all headers, footers,... several times. This is mostly useful for .txt files that were converted to PDF. In a catalogue or instruction set, Select all Text on Current Page is more useful. Thus, the idea to select the page is not worse than to select the whole document.
Considering headers and footers, these probably should be exclude-able from both Select All and Find..., The easiest way is probably to ignore the trimmed-away margins.
- Quadruple-click to select whole page What do other folks think?
In my view quadruple click does not make sense because it is seldom the user can think it can result in some kind of action or behaviour.
Secondly, this feature then would remain hidden or unknown about, even though I originally implemented this in double click. I would prefer double click over quadruple since for me double click is expected to do something, but surely quadruple is not.(I don't know about others)
At the end you all are much more experienced amd understand UX way better than me, I will leave this to you.
Double-click in the margins can't be used for this since there would be too many accidental activations.
Here's my proposal
- Double-click in the margins: select whole line
- Triple-click in the margins or on a word: select whole paragraph
- Quadruple-click in the margins: select whole page