Using QWebFrame::evaluateJavaScript does not relayout

[Richard Moore]

Created attachment 34700 [details]
Patch to fix the bug

It seems that executing javascript code using evaluateJavaScript that changes the visibility of some divs in a page does not cause an immediate relayout of the page. The page instead remains unchanged until the user moves the mouse over it. The problem was found by using js like this:

function toggleVisibility(id)
{
    el = document.getElementById(id);
    if (el.style.display == 'none')
    {
        el.style.display = '';
        window.silk.GM_log('showing');
    } else {
        el.style.display = 'none';
        window.silk.GM_log('hiding');
    }
}

toggleVisibility('sidebar');
toggleVisibility('header');
toggleVisibility('footer');

Executed against a gitorious repository. The same code executed in the inspector functions as expected. I've included what I think should be a patch to fix this issue in master, though since my code has kde dependencies against 4.5 I have been unable to test it so far.

[Simon Hausmann]

Comment on attachment 34700 [details]
Patch to fix the bug

This patch is missing a ChangeLog.

I'm not convinced it's the correct fix though. Why do the other ports appear not to need this?

For example the Mac API has stringByEvaluatingJavaScriptFromString, which can be called to evaluate javascript and the view will be updated afterwards. (A quick google search suggests that it works).

However I notice one important difference:

We are calling evaluate() on the ScriptController directly, whereas the mac port calls FrameLoader::executeScript, which calls Document::updateStyleForAllDocuments() after evaluation.

I think the correct fix involves calling executeScript instead of evaluate on the ScriptController.

[Richard Moore]

Ok, another patch that is closer to what Simon suggested.

[Richard Moore]

Created attachment 34786 [details]
A patch that changes how evaluateJavaScript works

[Richard Moore]

Created attachment 34897 [details]
Patch against 4.5.1 to fix the issue

This patch changes the way evaluateJavaScript works in QWebFrame  to use the FrameProxy. This fixes the bug. It also adds a test case for this issue.

[Richard Moore]

Created attachment 34898 [details]
Patch against master for the same issue

This patch makes the same change as the other one, but is for qt/master. The bug does not currently occur there, but using the frameproxy seems safer anyway. Either way the test case should be included to prevent the issue being reintroduced.

[Richard Moore]

I think the two patches I've attached are now suitable for inclusion to 4.5 branch of qt and master respectively.

[Richard Moore]

Created attachment 38735 [details]
Patch against Qt git repo

[Richard Moore]

Created attachment 38737 [details]
Patch against Qt git repo including changelog

[Richard Moore]

Created attachment 38738 [details]
Patch against qtwebkit git repo including changelog no tabs

[Richard Moore]

Created attachment 38739 [details]
Patch against qtwebkit git repo including changelog no tabs

[Tor Arne Vestbø]

Comment on attachment 34898 [details]
Patch against master for the same issue

Marking as obsolete, as patches should be made against webkit trunk (like the last one)

[Tor Arne Vestbø]

Landed in r47963