[Webkit-unassigned] [Bug 24300] Add CSSKeepVisitedLinksPrivate option to WebCore

[bugzilla-daemon at webkit.org]

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


abarth at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31746|review?                     |review-
               Flag|                            |




------- Comment #6 from abarth at webkit.org  2009-06-23 15:39 PDT -------
(From update of attachment 31746)
This is looking good!  Below are some minor style issues.  Once we address
those, we're all set.

> +function runTest() {
> +    if (window.layoutTestController) {
> +        layoutTestController.notifyDone();
> +    }
> +}

No braces around one-line if statements.

> +    Settings* settings = m_document->settings();
> +    if (!settings || settings->cssIgnoreVisitedLinks())
> +        m_IgnoreVisitedLinks = true;

If |settings| is null, then m_IgnoreVisitedLinks gets set to true, which is
isn't what we want (as I said in my previous review).

> @@ -262,7 +265,6 @@ QString DumpRenderTree::dumpFramesAsText(QWebFrame* frame)
>  
>  QString DumpRenderTree::dumpBackForwardList()
>  {
> -    QString result;
>      result.append(QLatin1String("\n============== Back Forward List ==============\n"));
>      result.append(QLatin1String("FIXME: Unimplemented!\n"));
>      result.append(QLatin1String("===============================================\n"));

I don't understand why you removed this line.  Is this just unrelated cleanup?

>      QWebPage *m_page;
> +    HistoryManager *historyManager;

These * are misplaced.  I see that you're matching the style of this file,
however.

> +HistoryManager::HistoryManager(QObject *parent)
> +    : QWebHistoryInterface(parent)
> +    ,m_count(0)

Missing a space after the , here.

> +    m_count++;

Prefer pre-increment (e.g, ++m_count).

Will this test fail on other platforms since you only implemented this API for
QT?  Maybe you should update the skipped test lists for those platforms to
avoid turning the tree red?


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