[Webkit-unassigned] [Bug 33617] [Qt] DRT missing setUserStyleSheetLocation and setUserStyleSheetEnabled in LayoutTestController

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33617





--- Comment #4 from Diego Gonzalez <diego.gonzalez at openbossa.org>  2010-01-13 14:03:11 PST ---
(In reply to comment #3)
> Let me add my 3 cents...
> 
> > --- a/WebKitTools/DumpRenderTree/qt/DumpRenderTreeQt.cpp
> > +++ b/WebKitTools/DumpRenderTree/qt/DumpRenderTreeQt.cpp
> > @@ -171,6 +171,7 @@ void WebPage::resetSettings()
> >      m_drt->layoutTestController()->setXSSAuditorEnabled(false);
> >  
> >      QWebSettings::setMaximumPagesInCache(0); // reset to default
> > +    settings()->setUserStyleSheetUrl(QUrl("")); //reset to default
> 
> The default constructor (QUrl()) would be fine as well.
> 
> > --- a/WebKitTools/DumpRenderTree/qt/LayoutTestControllerQt.cpp
> > +++ b/WebKitTools/DumpRenderTree/qt/LayoutTestControllerQt.cpp
> > @@ -391,3 +391,22 @@ void LayoutTestController::overridePreference(const QString& name, const QVarian
> >      else if (name == "WebKitUsesPageCachePreferenceKey")
> >          QWebSettings::setMaximumPagesInCache(value.toInt());
> >  }
> > +
> > +static QUrl userStyleSheetLocation = QUrl("");
> 
> I would make it a member variable instead.
> 
> > +void LayoutTestController::setUserStyleSheetLocation(const QString& url)
> > +{
> > +    if (!url.isEmpty())
> 
> Is this check even needed?
> 
> > +void LayoutTestController::setUserStyleSheetEnabled(bool enabled)
> > +{
> > +    QWebSettings* settings = m_topLoadingFrame->page()->settings();
> > +    if (settings) {
> 
> I doubt settings can be null here.
> 
> > +        if (enabled)
> > +            settings->setUserStyleSheetUrl(userStyleSheetLocation);
> > +        else
> > +            settings->setUserStyleSheetUrl(QUrl(""));
> 
> Same as above, I would use the default constructor.

Good suggestions! Thanks a lot. Reviewing now

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.