[Merge] lp:~abreu-alexandre/webbrowser-app/add-scheme-filter-handler into lp:webbrowser-app

Alexandre Abreu alexandre.abreu at canonical.com
Thu Aug 20 17:59:58 UTC 2015


> Docstrings in scheme-filter.h are outdated, and to be honest not very useful
> as they don’t contain any actual documentation. Can they be either updated, or
> removed?
> 
> On a related note, some minimal documentation at the class-level to explain
> the general purpose of the class and how it is used in conjunction with other
> constructs by the webapp container would be very helpful, especially for our
> future selves.
> 
> intent-parser.h has some unused includes (QObject, QVariantMap). The include
> for QUrl could be made a forward declaration.
> 
> intent-parser.cpp has some unused includes (QJSEngine, QJSValue).
> 
> The copyright notice for the two new files should add 2015 (it’s not new code,
> but those are new files).

> In test_default_scheme_filter, shouldn’t the test filter use
> encodeURIComponent() instead of r.path.replace(…) ?

all updated,

> In webapp-container.cpp, won’t the renaming of the default filename for the
> scheme filter (LOCAL_SCHEME_FILTER_FILENAME) break existing filters for
> already published apps?

Last time I looked no app uses a default filter for intents, the default one
does the work already,
-- 
https://code.launchpad.net/~abreu-alexandre/webbrowser-app/add-scheme-filter-handler/+merge/265265
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list