Closed
Bug 1396965
Opened 7 years ago
Closed 7 years ago
Bookmark star button is not displayed on about: pages
Categories
(Firefox :: Settings UI, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | --- | verified |
People
(Reporter: giul.mus, Assigned: adw)
References
Details
(Whiteboard: [reserve-photon-structure])
Attachments
(1 file)
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170904144521
Steps to reproduce:
Type `about:preferences` in the address bar.
Actual results:
No favourite button is shown.
Expected results:
The favourite button should have been shown.
Updated•7 years ago
|
Component: Untriaged → Preferences
Comment 1•7 years ago
|
||
[Tracking Requested - why for this release]: Regression
Oddly, mouse menu works fine, as you can favorite these internal pages.
Status: UNCONFIRMED → NEW
status-firefox57:
--- → affected
tracking-firefox57:
--- → ?
Ever confirmed: true
Keywords: regression
Yup. Ctrl+D also works, but displays the bookmark dialog to the left rather than to the right. I'll post screenshots soon.
Updated•7 years ago
|
Blocks: 1388835
Severity: minor → normal
status-firefox55:
--- → unaffected
status-firefox56:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Keywords: ux-consistency,
ux-control
OS: Unspecified → All
Hardware: Unspecified → All
Summary: "Favourite" button is not displayed on about:settings → Bookmark star button is not displayed on about: pages
Comment 3•7 years ago
|
||
Looks like its intended by bug #1388835,
but maybe we should exclude Bookmark star button from this ban on all about: pages (except about:newtab & about:blank to prevent errors, if users will try to bookmark page too fast and URL it not loaded yet)
tracking-firefox57:
? → ---
Keywords: regression,
ux-consistency,
ux-control
Updated•7 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(adw)
I can understand why one would want not to bookmark about:blank or about:newpage; less so for settings, which I might want to access quickly and frequently (eg. to change proxy settings).
Comment 5•7 years ago
|
||
Note that even if we revert bug 1388835, which I think might make sense, about:home will be unaffected as it was taken care of independently in bug 1393802.
Updated•7 years ago
|
Whiteboard: [photon-structure][triage]
Comment 6•7 years ago
|
||
I assumed the suggestion came from UX, I just reviewed the implementation.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(abenson)
Comment 7•7 years ago
|
||
note that for about:home|blank|newtab we could even have a startup perf win if we don't show the star (AND we also properly make it not update its state, something we are not currently doing).
Comment 8•7 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #7)
> note that for about:home|blank|newtab we could even have a startup perf win
> if we don't show the star (AND we also properly make it not update its
> state, something we are not currently doing).
But as I hinted at in comment 13, this bug should not be concerned with these pages. about:newtab didn't have the star even before bug 1388835 and about:home is treated like about:newtab as of bug 1393802.
Comment 9•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #8)
> (In reply to Marco Bonardo [::mak] from comment #7)
> > note that for about:home|blank|newtab we could even have a startup perf win
> > if we don't show the star (AND we also properly make it not update its
> > state, something we are not currently doing).
>
> But as I hinted at in comment 13
comment 5 even
Comment 10•7 years ago
|
||
While I can't imagine a lot of folks would do this, I can see a case for wanting to bookmark a preferences page so we should show the bookmark star but NOT the Page Action Menu.
Flags: needinfo?(abenson)
Updated•7 years ago
|
Flags: qe-verify?
Priority: -- → P4
Whiteboard: [photon-structure][triage] → [reserve-photon-structure]
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → adw
Status: NEW → ASSIGNED
Flags: needinfo?(adw)
Assignee | ||
Comment 11•7 years ago
|
||
(In reply to Aaron Benson from comment #10)
> While I can't imagine a lot of folks would do this, I can see a case for
> wanting to bookmark a preferences page so we should show the bookmark star
> but NOT the Page Action Menu.
I'll revert the change for the bookmark star but not the other buttons then.
Updated•7 years ago
|
Iteration: --- → 57.3 - Sep 19
Priority: P4 → P1
Comment 12•7 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #11)
> (In reply to Aaron Benson from comment #10)
> > While I can't imagine a lot of folks would do this, I can see a case for
> > wanting to bookmark a preferences page so we should show the bookmark star
> > but NOT the Page Action Menu.
>
> I'll revert the change for the bookmark star but not the other buttons then.
Awesome, thank you very much!
I have small suggestion to patch,
like Marco Bonardo [::mak] already wrote in Comment #7,
please exclude showing bookmark star button on:
- about:blank
- about:newtab
As there is no need to bookmark:
- about:blank - per its useless as bookmark (of course it could be accessible as about:newtab replacement by "+" (New Tab) button with extension)
- about:newtab - it's accessible by "+" (New Tab) button
This will prevent silly users errors, like mine in past - see bug #1371922,
where I wanted to bookmark some website page too fast with no URL loaded yet,
so in end I bookmarked about:blank/newtab by mistake.
I'm not fan of also banning about:home,
as user could have other not default Firefox page on Home button,
so to fast access about:home, bookmark placed in Bookmark Toolbar could be useful .
Assignee | ||
Comment 13•7 years ago
|
||
Aaron, one more q. People in this bug (or one person at least) have suggested that about:home should show the bookmark star. It did show the star until recently. Dao has said in this bug (comment 5, comment 8) that bug 1393802 meant that the star should appear or not appear the same in both about:home and about:newtab, and since newtab doesn't show the star, home shouldn't either. I looked through that bug and the referenced bug 1390359 and the Invision, but I don't see anything about that. Should the star appear on about:home?
Flags: needinfo?(abenson)
Comment 14•7 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #13)
> Should the star appear on about:home?
As Marco said in comment 7, not showing the star for about:home would be nice for startup performance, as it would let us delay Places initialization in the general case. In my opinion, the potential performance win outweighs the edge case mentioned in comment 12. Especially when the current plan is to have activity stream on about:home, making it equivalent to about:newtab from a user point of view.
Comment 15•7 years ago
|
||
Is there any data by how much Firefox is faster without showing bookmark star and other page actions buttons on pages like about:home?
Comment 16•7 years ago
|
||
(In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see your comment/reply/question/etc.) from comment #15)
> Is there any data by how much Firefox is faster without showing bookmark
> star and other page actions buttons on pages like about:home?
It's only about the first page that loads when starting the browser, not "pages like about:home". Showing the star requires having access to Places, which means loading the Places database off the disk, which can take more than a second (as any main thread IO, the time it takes is unpredictable, and potentially long if the disk is busy... which it often is during startup).
Comment 17•7 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #13)
> Aaron, one more q. People in this bug (or one person at least) have
> suggested that about:home should show the bookmark star. It did show the
> star until recently. Dao has said in this bug (comment 5, comment 8) that
> bug 1393802 meant that the star should appear or not appear the same in both
> about:home and about:newtab, and since newtab doesn't show the star, home
> shouldn't either. I looked through that bug and the referenced bug 1390359
> and the Invision, but I don't see anything about that. Should the star
> appear on about:home?
No star on about:home. From a UX perspective it doesn't make a whole lot of sense because there's the Home button for this.
Flags: needinfo?(abenson)
Comment 18•7 years ago
|
||
Which is to say, the home button provides a quick way back to this page aside from opening a new window/tab and bookmarking it is not needed/redundant.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 20•7 years ago
|
||
This shows the star on all the about pages for which bug 1388835 hid it and the other action buttons. It hides it on about:home, and the reason that works is because #urlbar[pageproxystate=invalid] now for about:home.
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8905585 [details]
Bug 1396965 - Bookmark star button is not displayed on about: pages.
https://reviewboard.mozilla.org/r/177390/#review182400
Makes sense, thanks!
Attachment #8905585 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 22•7 years ago
|
||
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bb2e73cf9473
Bookmark star button is not displayed on about: pages. r=Gijs
Comment 23•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 24•7 years ago
|
||
I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 57.0a1 (2017-09-08), so I'm marking this bug as VERIFIED. Thanks.
Updated•7 years ago
|
Flags: qe-verify?
You need to log in
before you can comment on or make changes to this bug.
Description
•