Review Request 116559: Provide a proxy javascript can call to say when its finished initialising its part of the AdiumThemeView. Fixes 325183

[Diane Trout]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116559/
-----------------------------------------------------------

(Updated March 4, 2014, 10:21 p.m.)


Review request for Telepathy.


Changes
-------

Bug https://bugs.kde.org/show_bug.cgi?id=319883 seemed related so I thought I'd try to see if I could fix it. This particular patch is more a proof of concept as I commented out the code in AdiumViewTheme::initialise that hooked the onload DOM event and just manually inserted the call into Template.html.

I did that because ChatWidget::initChatArea() calls AdiumThemeView::load and then AdiumThemeView::initialise so it seems like the template was getting read and used before initialise could modify the hook. I tried moving ::load after initialise and got a segfault in getTemplateHtml.

I'd love some suggestions about where to insert the onload event modification code.


Bugs: 325183
    http://bugs.kde.org/show_bug.cgi?id=325183


Repository: ktp-text-ui


Description
-------

What I discovered is that occasionally the AdiumThemeView will fire its loadFinished event before the body has finished loading. (which also means that some of the javascript functions don't get defined as they're created from the onLoad property of body.

What this patch does is injects a small proxy QObject (AdiumThemeViewProxy) into the javascript environment in AdiumThemeView::injectProxyIntoJavascript(). The proxy gives javascript access to a signal that it can call at the end of a new "configureEnvironment" javascript function. 

I added the new function instead of adding the signal to the end of the current initStyles functions because that appears to be overridable by some of the themes.

Also since this now lets you definitavely know that javascript is ready to append your messages, I removed some extra sendDemoMessages calls. 

I'm not sure what happens if a theme tries to override data/Template.html though.

Also I stuck AdiumThemeViewProxy in with the AdiumThemeView because it seemed small and tightly coupled with AdiumThemeView. However it seems like the convention is every class gets its own .h/.cpp file. So that might need to be fixed.


Diffs (updated)
-----

  lib/chat-widget.cpp e7c7619 
  lib/adium-theme-view.h 8d17f9d 
  lib/adium-theme-view.cpp 635a877 
  data/Template.html 6549479 
  config/appearance-config-tab.cpp ffef4f6 

Diff: https://git.reviewboard.kde.org/r/116559/diff/


Testing
-------

Set message style to Renkoo for both normal chat and group chat.

closed ktp-text-ui. 
reloaded ktp-text-ui. 
loaded configuration dialog. Clicked on both tabs. checked that they both rendered
close configuration dialog.
repeat opening the configuration dialog box and checking both tabs a few times.

Before the patch usually at least one of the tabs wouldn't render. It was somewhat random which one wouldn't render, and occasionally they both would render correctly.

I tried multiple times post patch and have yet to see it fail.


Thanks,

Diane Trout

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140305/559bdeb9/attachment.html>