1168960 - [User Story] Pin Site

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Browser Chrome, defect)

Description

[Ben Francis [:benfrancis]]

As a user I want to pin a web site so that I can re-use it often.

Comment 1

[ffos-browser]

A Pivotal Tracker story has been created for this Bug: https://www.pivotaltracker.com/story/show/95560446

Comment 2

[Ben Francis [:benfrancis]]

Attachment: Pin Site - Specification

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] benfrancis:1168960 > mozilla-b2g:master

Comment 4

[Ben Francis [:benfrancis]]

Attached WIP patch

Comment 5

[Ben Francis [:benfrancis]]

Comment on attachment 8647645 [details] [review]
[gaia] benfrancis:1168960 > mozilla-b2g:master

Hi Alberto, could you take a look at this?

This patch changes the existing pin button to pin a site instead of a page (the pin page code will be hooked up to some new UI).

If a manifest is provided then it's saved along with the siteObject in the bookmark store. It's currently setting siteObject.icon to be backwards compatible with verticalhome, but hopefully the new homescreen will be smarter about picking the right size icon using IconsHelper.

Comment 6

[Ben Francis [:benfrancis]]

Comment on attachment 8647645 [details] [review]
[gaia] benfrancis:1168960 > mozilla-b2g:master

Flagging Kevin in case he can get to it before Alberto can.

Comment 7

[Alberto Pastor [:albertopq]]

Thanks Ben. The code looks good (left minor comments on Github), but I have some questions:

- Should this bug include the UI for pinning the Site? Right now we still see 'Pin Page' and the preview card on the door hanger. Looks a little bit confusing at the moment.

- I'm still not convinced about using the current url (instead of the root host name) when pinning a site. I understand that sites like google.com/maps will pin google.com, but do we have many other cases in which pinning the root host name is a problem? How is different then pinning a site than a page if start_url/scope are not provided (which are most of the cases right now)? That said, this is just out of curiosity. This discussion won't block the r+ at all, obviously.

Thanks!

Comment 8

[Ben Francis [:benfrancis]]

Thanks Alberto,

(In reply to Alberto Pastor [:albertopq] from comment #7)
> - Should this bug include the UI for pinning the Site? Right now we still
> see 'Pin Page' and the preview card on the door hanger. Looks a little bit
> confusing at the moment.

Preview Pin Badge is bug 1168955 which I think Michael has nearly finished. I was going to wait for bug 1168958 to land but with the design changes we don't need that any more which is why it's a bit confused, we're swapping the pin page UI for the pin site UI. At least it's behind a pref.

> - I'm still not convinced about using the current url (instead of the root
> host name) when pinning a site. I understand that sites like google.com/maps
> will pin google.com, but do we have many other cases in which pinning the
> root host name is a problem? How is different then pinning a site than a
> page if start_url/scope are not provided (which are most of the cases right
> now)? That said, this is just out of curiosity. This discussion won't block
> the r+ at all, obviously.

Yeah this is tricky. Ideally I would like to use the root of the domain, but in the case of domains which are split into multiple sites that would make it impossible to pin those sites separately. The obvious example is Google properties, many of which are subdirectories of google.com but are clearly separate sites. Another tricky example is Evernote where evernote.com/ is their product web page and the web app starts at evernote.com/Home.action so you wouldn't want to use "/" as the start_url of the app.

We did a study of application scopes during the standardisation of the Web Manifest which shows you how messy URL spaces can be on the web https://docs.google.com/document/d/1fOsQWOOVuKyqO7cXZoKmxZGQ9FLgLMwmCRw3OEqIKrQ/edit?usp=sharing Many websites even span multiple domains which we don't take into account.

The difference between pinning a page and pinning a site is not that you're just pinning a different page. Pinning a site pins the whole URL scope which includes multiple pages, and pins the browser chrome to the status bar to make it behave more like an app (with the redesign we will no longer be pinning the browser chrome for pinned pages, as you suggested).

We have to default to using the current page as the start_url if none is provided because otherwise it would be impossible to pin some sites. It's possible we could change this later if we made it possible for the user to pick or edit the start_url, but that isn't part of the MVP.

Comment 9

[Alberto Pastor [:albertopq]]

Comment on attachment 8647645 [details] [review]
[gaia] benfrancis:1168960 > mozilla-b2g:master

Thanks for the explanation, Ben. Let's get this landed!

Comment 10

[Ben Francis [:benfrancis]]

Comment on attachment 8647645 [details] [review]
[gaia] benfrancis:1168960 > mozilla-b2g:master

Addressed review comments, waiting for the tree to re-open in order to land.

Comment 11

[Ben Francis [:benfrancis]]

This didn't land due to tree closures and then bitrotted. I've rebased it but there are some test failures I can't reproduce locally.

Still investigating.

Comment 12

[GitHub Autolander Bot]

Attachment: [gaia] benfrancis:1168960-4 > mozilla-b2g:master

Comment 13

[Ben Francis [:benfrancis]]

https://github.com/mozilla-b2g/gaia/commit/1ceb1b72b8c945894e1198202080a8dcc49a07b3