[Webkit-unassigned] [Bug 52296] [Qt]Add local storage settings to QtTestBrowser option menu

[bugzilla-daemon at webkit.org]

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


Andreas Kling <kling at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #79195|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #9 from Andreas Kling <kling at webkit.org>  2011-01-17 13:22:14 PST ---
(From update of attachment 79195)
View in context: https://bugs.webkit.org/attachment.cgi?id=79195&action=review

> Tools/QtTestBrowser/launcherwindow.cpp:277
> +    QAction* toggleOfflineWebApplicationCache = toolsMenu->addAction("Enable Offline WebApplication Cache", this, SLOT(toggleOfflineWebApplicationCache(bool)));

"WebApplication" -> "Web Application"

> Tools/QtTestBrowser/launcherwindow.cpp:281
> +    QAction* offlineStorageDefaultQuotaAction = toolsMenu->addAction("Set Offline Storage Default Quota Maxsize", this, SLOT(setOfflineStorageDefaultQuota())); 

"Maxsize" -> "Max Size"

> Tools/QtTestBrowser/launcherwindow.cpp:283
> +    offlineStorageDefaultQuotaAction->setChecked((m_windowOptions.offlineStorageDefaultQuotaSize) ? true : false);

The "? true : false" is unnecessary.

> Tools/QtTestBrowser/launcherwindow.cpp:286
> +    

Whitespace.

> Tools/QtTestBrowser/launcherwindow.cpp:787
> +void LauncherWindow::toggleLocalStorage(bool toggle)

"toggle" is called "enable" in the declaration (and in the similar function above.) We should be consistent.

> Tools/QtTestBrowser/launcherwindow.cpp:793
> +void LauncherWindow::toggleOfflineStorageDatabase(bool toggle)

Ditto.

> Tools/QtTestBrowser/launcherwindow.cpp:799
> +void LauncherWindow::toggleOfflineWebApplicationCache(bool toggle)

Ditto.

> Tools/QtTestBrowser/launcherwindow.cpp:807
> +    // for command line execution, quota size is taken from command line and set it below    

Comments should start with a capital and end with a period.

> Tools/QtTestBrowser/launcherwindow.cpp:809
> +       page()->settings()->setOfflineStorageDefaultQuota(m_windowOptions.offlineStorageDefaultQuotaSize);

Coding style, indentation should be 4 spaces, not 3 (this block) or 2 (the block below.)

> Tools/QtTestBrowser/launcherwindow.cpp:811
> +      QDialog* dialog = new QDialog(this);

This variable should be on the stack.

> Tools/QtTestBrowser/launcherwindow.cpp:814
> +      dialog->setWindowTitle("Offline Storage Default Quota Maxsize");

Strange title, let's do "Maxsize" -> "Max Size" at least.

> Tools/QtTestBrowser/launcherwindow.h:121
> +    bool useLocalStorageEnabled;
> +    bool useOfflineStorageDatabaseEnabled;
> +    bool useOfflineWebApplicationCacheEnabled;
> +    quint64 offlineStorageDefaultQuotaSize;    

These variables should be initialized (in the WindowOptions constructor above.)
Also, the "Enabled" suffix on the first 3 is redundant.

> Tools/QtTestBrowser/launcherwindow.h:185
> -
> +    

Whitespace.

> Tools/QtTestBrowser/main.cpp:168
> +         

Whitespace.

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