Code review comment for lp:~rpadovani/webbrowser-app/settings-page

> Indeed it works. I know it isn't a good practice to mix code that should
> belong to a different branch, but since it's an half line change, I added also
> a flag for the history view - of course, if you don't agree, I remove that

Very good initiative!

> > - Clicking "Reset browser settings" when a value has just been changed
> won’t
> > update the corresponding entry in the view (try e.g. to change the homepage
> > and just after that reset the settings)
>
> I'm totally lost on this one - I tried with all possible combination of
> browser.property and setting.property in Browser.qml, but nothing works.
>
> Seems updating the property breaks the binding - only the view is broken, not
> the function itself.
>
> Maybe after some sleep I'll be able to fix that, but every suggestion is
> welcome!

I’ll take a closer look now.

> > - In the visual spec, it looks like all items have a thin divider at the
> > bottom, so the "Reset browser settings" one probably needs one as well
>
> I added it, but I totally don't like it: it's a divider, if there isn't
> anything to divide 'cause it's the last element, why do we have to add it? :-)

I don’t like it either, but let’s stick to the visual spec, it’s easy enough to change it later on.

> Also, I implemented the search engine choice, works as a charm (at least with
> DuckDuckGo).
> Your solution is very elegant and very simple - I just have to don't think how
> much time I spent on C++ model (actually, I learned a lot, so I'm happy) :-)
>
> I didn't add yet the search engine files, which ones do we want?
>
> Could I suggest Google, Bing, Yahoo and DuckDuckGo?

Yeah, I’d like to add more search engines to the list by default, but that will require more work as we would probably need to monitor several folders (system-wide + locally installed by the user), so let’s do that separately.