Browser-laptop: Remove customTitle from siteDetail object

[bsclifton]

Test plan

https://github.com/brave/browser-laptop/pull/10136#issue-245629798

---

Discussion starts here https://github.com/brave/browser-laptop/issues/6108#issuecomment-265934841

To keep things simple, I'd like to propose that we eliminate custom title.

• When a bookmark is added for a page, it will default to the title of the page
• Once set, the title for that bookmark will never be updated again

Removing this should resolve the following issues:
https://github.com/brave/browser-laptop/issues/6104
https://github.com/brave/browser-laptop/issues/6108
https://github.com/brave/browser-laptop/issues/3694

There are likely more which have this as the root cause (we can search for issues where titles change)

[bbondy]

I think removing custom title is wrong.
If you did...
If you rename a bookmark, then you visit a site do you update the title attribute?
If you don't update the title attribute, what if the website title changes?

[bbondy]

I think if we did we would have to split out bookmarks from sites at the same time

[bbondy]

I think we should do it but I think we should split out bookmarks first or at the same time.

[bsclifton]

@bbondy from the original discussion (sorry, should have posted this here too- @bradleyrichter had the same concern)

@bradleyrichter my proposal is to eliminate the custom title field and then use the regular title field like a custom title. Right now, the title is something that users can't change... but it causes problems because it will update if the page title updates. The user would have to go and explicitly set a title if they didn't want it to change

I think it's a better experience to always treat the title like it's a custom title. It should never be updated unless the person changes it themselves. That should be consistent with other browsers. What do you think?

[bsclifton]

@bbondy the title updating itself is what causes several bugs unfortunately. For example, let's say the site ends up doing a redirect via JavaScript with a simple "we've moved" page. In this case, it'll rename the page "We've moved! Redirecting you now to so and so.com...."

I tested with Safari and Chrome briefly and once a bookmark is saved, the title did not seem to be updated when visited again (if the page title was updated)

[bbondy]

If the title of a bookmark is updating currently on navigation then a bug was introduced, this used to work correctly. Still does as far as I can tell from testing just now.

I'm ok and in favour of getting rid of customTitle, it's only wrong if done without any other changes. It should be done with splitting out bookmarks from within the sites storage. This will also solve problems with the upgrade scenario if we simply stopped using customTitle. It will also simplify the logic in various places.

• visitedSites: sites that have no tags
• bookmarks: sites that have bookmark or bookmark-folder tags, they need to be stored together because you can reorder them on the bookmarks toolbar
• pinnedSites: sites with pinned tag

The reader tag describe in docs/state.md is not used so can be ignored.

[bbondy]

Also you can still use the siteUtils for each of these but just store them differently in app storage.
Upgrade scenario can just check if appState has a sites, and if so then do the upgrade steps to split them. App would then use the 3 new categories directly and not sites.

[bsclifton]

Great points @bbondy :smile:

@NejcZdovc you can check the documention by @cezaraugusto for our existing "Site Detail" objects and behavior here:
https://github.com/brave/browser-laptop/wiki/Overview-of-the-siteDetail-object

And @darkdh has modified that behavior further with https://github.com/brave/browser-laptop/pull/5587 (making each entry a hash instead, which is unique; PR is still open- pls feel free to review). Like @bbondy is saying, this is a great chance to remove bookmarks from the sites object :smile:

Let's talk more on Slack if you have questions or want to talk about a design (we can then document any findings here)

[NejcZdovc]

Blocked by #5587

[ayumi]

I think splitting sites out is a good idea. We'll need to update Sync code to match the new schema.

cc @diracdeltas

[diracdeltas]

i agree

[luixxiul]

@NejcZdovc is this issue still blocked on something? https://github.com/brave/browser-laptop/issues/6585#event-980859653

[NejcZdovc]

No it's not blocked anymore

[NejcZdovc]

Will be fixed with #10136