----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116559/#review51925 -----------------------------------------------------------
Ship it! lib/adium-theme-view.cpp <https://git.reviewboard.kde.org/r/116559/#comment36918> document this so that future you can understand it and why. Especially the regex. No-one likes reverse engineering them. Can you make sure you test: - the log viewer - changing the theme during an active chat - David Edmundson On March 4, 2014, 5:25 a.m., Diane Trout wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116559/ > ----------------------------------------------------------- > > (Updated March 4, 2014, 5:25 a.m.) > > > Review request for Telepathy. > > > 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 > ----- > > lib/adium-theme-view.h 8d17f9d > lib/adium-theme-view.cpp 635a877 > 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 > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
