Bug 36451 – [Qt] User Agent Switcher on QtLauncher

• Status: CLOSED
• Resolution: FIXED
• Component: WebKit Qt

[Diego Gonzalez]

Make possible QtLauncher access pages that restrict access based
on the browser being used and/or simply test the browser with
different user agents.

[Diego Gonzalez]

Created attachment 51302 [details]
user agent dialog

[Diego Gonzalez]

Created attachment 51303 [details]
user agents predefined list

[Diego Gonzalez]

Created attachment 51304 [details]
edit manually the user agent string

If the edited string is not in the user agents list it will be stored

[Diego Gonzalez]

Created attachment 51309 [details]
Proposed patch

[Simon Hausmann]

Comment on attachment 51309 [details]
Proposed patch


> +void LauncherWindow::showUserAgentDialog()
> +{
> +    QFile file("WebKitTools/QtLauncher/useragentlist.txt");

Instead of requiring that QtLauncher is started from the top-level directory by
using a relative path here, I think it would be better to compile the user agent list
into the application using a Qt resource.

> +    , m_userAgent(QString())

This initialization isn't necessary.

> +    void setUserAgent(QString ua) { m_userAgent = ua; }

The parameter should be a const QString&.

r- until the above issues are addressed. The rest looks good to me! It's great that you're adding this, it really helps for development/testing.

[Diego Gonzalez]

(In reply to comment #5)
> (From update of attachment 51309 [details])
> 
> > +void LauncherWindow::showUserAgentDialog()
> > +{
> > +    QFile file("WebKitTools/QtLauncher/useragentlist.txt");
> 
> Instead of requiring that QtLauncher is started from the top-level directory by
> using a relative path here, I think it would be better to compile the user
> agent list
> into the application using a Qt resource.
> 

Thanks for the feedback Simon!

Using Qt resource is a good way if I want to only read the data from useragentlist.txt, because afaik we only can open it in ReadOnly mode, so is not possible to store a new user agent in useragentlist at runtime and is needed to recompile if we want the edit the list manually. Do you know if is possible do it usinf Qt Resource or do you have other suggestion? :)

[Diego Gonzalez]

Created attachment 51440 [details]
Proposed patch

Changed according Simon's suggestions

[Diego Gonzalez]

> Using Qt resource is a good way if I want to only read the data from
> useragentlist.txt, because afaik we only can open it in ReadOnly mode, so is
> not possible to store a new user agent in useragentlist at runtime and is
> needed to recompile if we want the edit the list manually. Do you know if is
> possible do it usinf Qt Resource or do you have other suggestion? :)

I am using now QSettings to store and keep the data persistence :)

[Diego Gonzalez]

Created attachment 51518 [details]
Proposed patch

Simplifying the patch to only read and show the user agents from the 
predefined list for this moment.

[Daniel Bates]

Created attachment 51541 [details]
Safari Develop->User Agent menu

Instead of a combined list and (custom user agent text field)?, I would suggest that we add the sub-menu User Agent to the Develop menu in QtLauncher and that it has at least the same options and behavior as the identical menu found in Safari. This will make QtLauncher consistent with Safari. Notice, the last option in the Safari User Agent menu is "Other..." which opens a dialog with an text field that is initially populated with the value of the currently active user agent string.

[Daniel Bates]

Created attachment 51542 [details]
Safari Develop->User Agent->Other

[Diego Gonzalez]

(In reply to comment #10)
> Created an attachment (id=51541) [details]
> Safari Develop->User Agent menu
> 
> Instead of a combined list and (custom user agent text field)?, I would suggest
> that we add the sub-menu User Agent to the Develop menu in QtLauncher and that
> it has at least the same options and behavior as the identical menu found in
> Safari. This will make QtLauncher consistent with Safari. Notice, the last
> option in the Safari User Agent menu is "Other..." which opens a dialog with an
> text field that is initially populated with the value of the currently active
> user agent string.

I thing it can be considered. Kenneth? Simon?

[Kenneth Rohde Christiansen]

Sounds fine with me!

[Simon Hausmann]

Absolutely! :)

Please create a new bugzilla entry for the custom UA entry

[Simon Hausmann]

Comment on attachment 51518 [details]
Proposed patch


> +void LauncherWindow::showUserAgentDialog()
> +{
> +    QStringList items;
> +    QFile file(":/useragentlist.txt");
> +    if (file.open(QIODevice::ReadOnly)) {
> +         while (!file.atEnd())
> +            items << file.readLine().trimmed();
> +        file.close();
> +    }
> +
> +    QDialog* dialog = new QDialog(this);
> +    dialog->setWindowTitle("Change User Agent");
> +
> +    QVBoxLayout* layout = new QVBoxLayout(dialog);
> +    dialog->setLayout(layout);
> +
> +    QComboBox* combo = new QComboBox(dialog);
> +    combo->setMaximumWidth(size().width() * 0.7);
> +    combo->insertItems(0, items);
> +    layout->addWidget(combo);
> +
> +    QDialogButtonBox* buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok
> +            | QDialogButtonBox::Cancel, Qt::Horizontal, dialog);
> +    connect(buttonBox, SIGNAL(accepted()), dialog, SLOT(accept()));
> +    connect(buttonBox, SIGNAL(rejected()), dialog, SLOT(reject()));
> +    layout->addWidget(buttonBox);
> +
> +    if (dialog->exec() && !combo->currentText().isEmpty())
> +        page()->setUserAgent(combo->currentText());
> +}
> +

Please delete the dialog object after use.

Otherwise r=me :)

[Diego Gonzalez]

Landed by tonikitoo at r56636

> Please delete the dialog object after use.
> 
> Otherwise r=me :)

[Simon Hausmann]

cherry-pick-for-backport: <r56636>

[Simon Hausmann]

Revision r56636 cherry-picked into qtwebkit-2.0 with commit 0b6b1ebafb42ed11323faf42afb5eaa4b2ef1df1