Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Bookmark title on bookmark toolbar is renamed #6108

Closed
luixxiul opened this issue Dec 9, 2016 · 12 comments
Closed

Bookmark title on bookmark toolbar is renamed #6108

luixxiul opened this issue Dec 9, 2016 · 12 comments

Comments

@luixxiul
Copy link
Contributor

luixxiul commented Dec 9, 2016

Test plan

#10136 (comment)


Describe the issue you encountered:
Bookmark title on the bookmark toolbar is renamed after clicking the link to the notification page on GitHub.

Expected behavior:
The bookmark title should not be changed.

  • Platform (Win7, 8, 10? macOS? Linux distro?): macOS

  • Brave Version: master branch

  • Steps to reproduce:

    1. Bookmark the notification page on GitHub
    2. Click the notification link -> the bookmark title is changed
  • Screenshot if needed:
    bookmark

  • Any related issues:

@luixxiul luixxiul changed the title Bookmark title is renamed after Bookmark title on bookmark toolbar is renamed Dec 9, 2016
@luixxiul
Copy link
Contributor Author

luixxiul commented Dec 9, 2016

Edited.

@bsclifton
Copy link
Member

bsclifton commented Dec 9, 2016

This may or may not be expected. This is the current behavior:

  • When a site is visited, a siteDetail record will be created
  • If one already exists, that will be updated
  • The new title ALWAYS overwrites the old title. Custom title does not change
  • If custom title is never set (which only happens if user manually does it), the user will always see the title (which will end up being the new one, OR if blank, the URL)

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

What do you think @luixxiul? Also, CC: @bradleyrichter @BrendanEich @bbondy @darkdh (and everyone else too 😄 )

@luixxiul
Copy link
Contributor Author

luixxiul commented Dec 9, 2016

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

If this is the default behavior on Chrome/Firefox, +1 :)

@cndouglas
Copy link

+1 for your proposal. It seems like the least confusing option from a user perspective.

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Dec 9, 2016

+1 When you are visiting a page that is also bookmarked, this should not change bookmark title. I would keep the same functionality for custom title as is now. Other browsers has the same functionality.

More problematic problem is that when you click on the same url on the page that you are already on, title is not rendered correctly. But if you click on this url in bookmark bar or refresh the page, title is rendered correctly. We can see this in @luixxiul screenshot.

@luixxiul
Copy link
Contributor Author

luixxiul commented Dec 9, 2016

@NejcZdovc I have reported the issue on #5738

@bsclifton
Copy link
Member

@luixxiul would this be something you'd be interested in looking at? (I'd be more than happy to help!) I bet there are at least 2 or 3 more issues related to this same root cause. Let me know 😄

@bradleyrichter
Copy link
Contributor

@bsclifton I think the custom title is a requirement. Quite often, a page is bookmarked at an entry point to a top domain but the user wants to simply the BM title to be the top domain.

For example:

image

I wanted to rename this obscure title (when truncated) to 'CSS Tricks" which is far more recognizable/meaningful.

image

image

@bsclifton
Copy link
Member

bsclifton commented Dec 9, 2016

@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?

@bradleyrichter
Copy link
Contributor

Yes. Agree. I was confused about the meaning of Title and Custom Title. The user only sees title.

Under the hood, we should just treat a title as a custom title and never auto-update any BM data. Only if the user updates the data manually. (as you described)

@NejcZdovc
Copy link
Contributor

If we remove CustomTitle then issue #6104 and PR #6111 can be closed.

@luixxiul luixxiul added this to the 1.2.0 milestone May 29, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 18, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 18, 2017
Resovles brave#9978
Resolves brave#6108
Resovles brave#6585
Resolves brave#6104
Resolves brave#3694

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 20, 2017
Resovles brave#9978
Resolves brave#6108
Resovles brave#6585
Resolves brave#6104
Resolves brave#3694

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 20, 2017
Resovles brave#9978
Resolves brave#6108
Resovles brave#6585
Resolves brave#6104
Resolves brave#3694

Auditors:

Test Plan:
@NejcZdovc NejcZdovc modified the milestones: 0.20.x (Developer Channel), 1.2.0 Jul 20, 2017
@NejcZdovc NejcZdovc self-assigned this Jul 20, 2017
@NejcZdovc
Copy link
Contributor

NejcZdovc commented Jul 20, 2017

Will be fixed in #10136

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 21, 2017
Resovles brave#9978
Resolves brave#6108
Resovles brave#6585
Resolves brave#6104
Resolves brave#3694

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 22, 2017
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 22, 2017
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 25, 2017
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 25, 2017
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 26, 2017
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Jul 27, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Jul 27, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Jul 27, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Jul 28, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Jul 28, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Jul 28, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Jul 31, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Jul 31, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Aug 1, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
NejcZdovc added a commit that referenced this issue Aug 3, 2017
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
@NejcZdovc NejcZdovc modified the milestones: 0.21.x (Nightly Channel), 0.20.x (Developer Channel) Aug 4, 2017
NejcZdovc added a commit that referenced this issue Aug 4, 2017
Resolves #1646
Resolves #1856
Resolves #2655
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5072
Resolves #5699
Resolves #5382
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
dfperry5 pushed a commit to dfperry5/browser-laptop that referenced this issue Aug 18, 2017
Resolves brave#1646
Resolves brave#1856
Resolves brave#2655
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5072
Resolves brave#5699
Resolves brave#5382
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
@bbondy bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel) Oct 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.