Merge lp://qastaging/~3v1n0/ubuntu-system-settings-online-accounts/webview-prefs into lp://qastaging/ubuntu-system-settings-online-accounts

Proposed by Marco Trevisan (Treviño)
Status: Approved
Approved by: Alberto Mardegan
Approved revision: 238
Proposed branch: lp://qastaging/~3v1n0/ubuntu-system-settings-online-accounts/webview-prefs
Merge into: lp://qastaging/ubuntu-system-settings-online-accounts
Diff against target: 66 lines (+15/-1)
3 files modified
plugins/module/OAuth.qml (+3/-1)
plugins/module/OAuthMain.qml (+8/-0)
plugins/module/WebView.qml (+4/-0)
To merge this branch: bzr merge lp://qastaging/~3v1n0/ubuntu-system-settings-online-accounts/webview-prefs
Reviewer Review Type Date Requested Status
Víctor R. Ruiz (community) Needs Fixing
Alberto Mardegan (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+249877@code.qastaging.launchpad.net

Commit message

OAuthMain: add ability to clients to change WebView preferences

Sometimes OAuthMain extenders might need to change the webview
preferences, this adds a webViewPreferences property that can
be filled with values for overriding defaults

Description of the change

I had to login to a service that needed the local storage to be available in the WebView when logging in, and this is not enabled by default in the account view.

I'm not sure whether is a good thing to enable that in the WebView for every account, so while I could just override the default OAuth.qml and WebView.qml in my app, this is not the best thing as it could lead to breakage on future changes.

So, I thought that it might be reasonable to add a webViewPreferences that every account that extends OAuthMain can easily change.

In this way I can get things working by just doing:

OAuthMain
{
  webViewPreferences: {"localStorageEnabled": true}
}

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Mardegan (mardy) wrote :

The code looks good, but do you have an example plugin to test this?

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :
Revision history for this message
Alberto Mardegan (mardy) wrote :

Tested, works fine.

review: Approve
Revision history for this message
Víctor R. Ruiz (vrruiz) wrote :

The new feature needs an automated test.

review: Needs Fixing
Revision history for this message
Alberto Mardegan (mardy) wrote :

Marco do you think you could add the test? You can use a fake Ubuntu.WebView and just check that the preferences are propagated to it.

If not, I'll do that, but it will take some time as this is not currently in uour high-priority list.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Alberto, ok. I'll look into it.

Are there already some tests that I can use as example?

Revision history for this message
Alberto Mardegan (mardy) wrote :

On 20.05.2015 12:19, Marco Trevisan (Treviño) wrote:
> Alberto, ok. I'll look into it.

Thanks! :-)

> Are there already some tests that I can use as example?

Not really, the closest thing I can think of is
tests/client/tst_qml_client.cpp, because I think you'll need a C++ test
which asks the QML engine to run some QML code.

Your test should be in tests/plugin/

Unmerged revisions

238. By Marco Trevisan (Treviño)

OAuthMain: add ability to clients to change WebView preferences

Sometimes OAuthMain extenders might need to change the webview
preferences, this adds a webViewPreferences property that can
be filled with values for overriding defaults

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches