Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QWebView - process selection before eval #173

Closed
jleben opened this issue May 6, 2012 · 4 comments
Closed

QWebView - process selection before eval #173

jleben opened this issue May 6, 2012 · 4 comments
Assignees
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead
Milestone

Comments

@jleben
Copy link
Member

jleben commented May 6, 2012

[Issue migrated from SourceForge | ID: 3307470 | Submitted by 'lijon']
[http://sourceforge.net/support/tracker.php?aid=3307470]

Before evaluating selected text, call JS selectLine() (defined in scdoc.js) to possibly select the current line.

@ghost ghost assigned jleben May 6, 2012
@jleben
Copy link
Member Author

jleben commented May 6, 2012

[Comment migrated from SourceForge | Submitted by 'jleben']

Keep in mind that QWebView can be used for other purposes than HelpBrowser, so invoking some JS whenever a key combination is pressed is not approriate in QWebView itself.

One solution is to expose evaluateJavaScript() method to SC language and use it in HelpBrowser.

Another solution is to defer the evaluation of selection after the key pressed is processed by the WebView and the line is selected by the web page's JS. I tested this and it works.

@jleben
Copy link
Member Author

jleben commented May 6, 2012

[Comment migrated from SourceForge | Submitted by 'jleben']

I would prefer exposing evaluateJavaScript() to SC language, as It would make a very useful feature anyway.

The method would evaluate some JS in the current frame - see QWebPage::currentFrame() and QWebFrame::evaluateJavaScript() documentation.

But that implies SCWebView has to do the same

@jleben
Copy link
Member Author

jleben commented May 6, 2012

[Comment migrated from SourceForge | Submitted by 'lijon']

Evaluating JS from SC would be useful for many purposes yes.

Invoking JS to call selectLine() would be done in the same code in QWebView that evaluates example code, so it is already specific. Would be better if it was HelpBrowser, not WebView, that handled the key-shortcut. And that the only thing WebView did was to provide the selected text.

Anyhow, if defering the evaluation after key has pressed works, then I think we should go with that for now and close this bug.

@jleben
Copy link
Member Author

jleben commented May 6, 2012

[Comment migrated from SourceForge | Submitted by 'jleben']

Calling JS selectLine() is not equivalent in specificity to the current code to evaluate selection.

Currently the selection evaluation is optional, plus it can only have results expected by the user: if they turn it on, they see exactly what SC code is executed when they press the shortcut.

On the other hand, blindly invoking selectLine() might throw a JS error if function does not exist (I guess), or even worse: might invoke a function of the same name in another web page, that will do something that the user can never expect and is kind of a security hole!

And I still prefer exposing evaluateJavaScript() to SC than defering the event, because the latter involves an ugly hack, because it can not expect that letting the WebView process the event will indeed set a selection: if there is already selection, evaluate it, otherwise let the webpage process the event, and then try to evaluate again.

So I vote for exposing evaluateJavaScript() to SC language

@jleben jleben closed this as completed May 6, 2012
mossheim pushed a commit that referenced this issue Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead
Projects
None yet
Development

No branches or pull requests

1 participant