Closed Bug 1477591 Opened Last year Closed Last year

Missing favicon placeholder in new all tabs panel if no Site/Fav icon is provided by the website

Categories

(Firefox :: Tabbed Browser, defect, P1)

63 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 + verified
firefox63 + verified

People

(Reporter: pevar, Assigned: mstriemer)

References

Details

(Keywords: nightly-community, regression)

Attachments

(6 files)

Attached image FF61 vs FF63
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180722100230

Steps to reproduce:

Disable site/fav icons and open few tabs and click list all tabs button


Actual results:

the list has empty space before tab title and empty space instead of default icons


Expected results:

Defaut site icons should be shown and there should not be any space before the title of tab starts
This regression is caused by 
Bug 1470947
thanks to mozregression
NI? owner
Flags: needinfo?(mstriemer)
Flags: needinfo?(mjaritz)
Blocks: 1470947
Severity: normal → major
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Tabbed Browser
OS: Unspecified → All
Hardware: Unspecified → All
Depends on: 1477594
Severity: major → normal
Priority: -- → P2
Not sure why this would be a big problem, as disabling favicons is only possible through about:config, bu if caused by our work, Mark might be able to fix it.
Flags: needinfo?(mjaritz)
No longer depends on: 1477594
(In reply to Markus Jaritz [:designakt] (UX) from comment #2)
> Not sure why this would be a big problem, as disabling favicons is only
> possible through about:config, bu if caused by our work, Mark might be able
> to fix it.

Yes this is caused after Bug 1470947 and/or Bug 1446101

By default a globe icon was shown by Firefox if site icon was missing, not provided by the site(happens alot) or disabled by the user.

Now IT's a large blank space so this is a regression.

Plus now there is a blank space before the site icon which is ugly too see as well as reduces the tab title visible which is
just bad UX.
There should be no blank space before Icons just like in FF60
see attachment.
Flags: needinfo?(mjaritz)
(In reply to Jason Mechelynck from comment #3)
> By default a globe icon was shown by Firefox if site icon was missing, not
> provided by the site(happens alot) or disabled by the user.
> 
> Now it's a large blank space so this is a regression.

Thanks for helping me understand this better. The case where there is no icon provided by the website seams to be the bigger one here, as it will be experienced by more people. I agree that this has a bad impact on the readability of the list, and should be filled with a placeholder. I updated the bug summary accordingly.

Mark, can you fix this?
Flags: needinfo?(mjaritz)
Summary: Visual regressions in new all tabs panel if Site/Fav icons disabled → Missing favicon placeholder in new all tabs panel if no Site/Fav icon is provided by the website
[Tracking Requested - why for this release]:
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Jason Mechelynck from comment #3)
> Plus now there is a blank space before the site icon which is ugly too see
This space helps separate foreground from background content. 

> as well as reduces the tab title visible which is just bad UX.
The width is not about showing all text, but ensuring a balance between line length, and the ease of identifying individual tabs.
The new width is set to show an average of 66 characters, which is the optimal line length for readability, and will also ensure that by far most website-titles will be easily differentiated. (If not all 66 characters are identical.)
There is no bad UX in that.
Attached image Clipboard01.jpg
(In reply to Markus Jaritz [:designakt] (UX) from comment #6)
> (In reply to Jason Mechelynck from comment #3)
> > By default a globe icon was shown by Firefox if site icon was missing, not
> > provided by the site(happens alot) or disabled by the user.
> > 
> > Now it's a large blank space so this is a regression.
> 
> Thanks for helping me understand this better. The case where there is no
> icon provided by the website seams to be the bigger one here, as it will be
> experienced by more people. I agree that this has a bad impact on the
> readability of the list, and should be filled with a placeholder. I updated
> the bug summary accordingly.
> 
> Mark, can you fix this?

Yes that is the problem and thank you.

(In reply to Markus Jaritz [:designakt] (UX) from comment #8)
> (In reply to Jason Mechelynck from comment #3)
> > Plus now there is a blank space before the site icon which is ugly too see
> This space helps separate foreground from background content. 
> 
> > as well as reduces the tab title visible which is just bad UX.
> The width is not about showing all text, but ensuring a balance between line
> length, and the ease of identifying individual tabs.
> The new width is set to show an average of 66 characters, which is the
> optimal line length for readability, and will also ensure that by far most
> website-titles will be easily differentiated. (If not all 66 characters are
> identical.)
> There is no bad UX in that.

The tab highlight is missing(it was full tab highlight) its now just a blue small line hard to locate at a glance.
The old all tab list showed more info than the current one plain and simple especially with disabling site icons which allowed more characters.
Flags: needinfo?(mjaritz)
(In reply to Jason Mechelynck from comment #9)
> The tab highlight is missing(it was full tab highlight) its now just a blue
> small line hard to locate at a glance.

I filed a bug for that, and it is about to be fixed:
https://bugzilla.mozilla.org/show_bug.cgi?id=1478708
Flags: needinfo?(mjaritz)
(In reply to Markus Jaritz [:designakt] (UX) from comment #10)
> (In reply to Jason Mechelynck from comment #9)
> > The tab highlight is missing(it was full tab highlight) its now just a blue
> > small line hard to locate at a glance.
> 
> I filed a bug for that, and it is about to be fixed:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1478708

Still not full tab highlight just Bold text is used
(In reply to Mark Galoway from comment #11)
> (In reply to Markus Jaritz [:designakt] (UX) from comment #10)
> > (In reply to Jason Mechelynck from comment #9)
> > > The tab highlight is missing(it was full tab highlight) its now just a blue
> > > small line hard to locate at a glance.
> > 
> > I filed a bug for that, and it is about to be fixed:
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1478708
> 
> Still not full tab highlight just Bold text is used

Then I probably do not understand what it is you mean with tab highlight. 
Are you talking about:
a) the highlight of the active tab 
   (which was bold text, and now is a blue line and bold text - soon)
b) the highlight of each row when hovered with the mouse or focused with the keyboard
   (which was blue, and now is gray background on the row)
c) something else
   (please specify)
Assignee: nobody → mstriemer
Flags: needinfo?(mstriemer)
Comment on attachment 8996058 [details]
Bug 1477591 - Use default favicon in all tabs menu

https://reviewboard.mozilla.org/r/260316/#review268234

::: browser/themes/shared/tabs.inc.css:803
(Diff revision 1)
>  
>  .all-tabs-item[selected] {
>    box-shadow: inset 4px 0 var(--blue-40);
>  }
>  
> +.all-tabs-button > .toolbarbutton-icon {

Just use .all-tabs-button without .toolbarbutton-icon? list-style-image gets inherited.
Attachment #8996058 - Flags: review?(dao+bmo) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 00573673f9aef1f353a1b07e42ec5ffc72f4820d -d 218ec331cdec: rebasing 476657:00573673f9ae "Bug 1477591 - Use default favicon in all tabs menu r=dao" (tip)
merging browser/themes/shared/tabs.inc.css
warning: conflicts while merging browser/themes/shared/tabs.inc.css! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc62d0350cbb
Use default favicon in all tabs menu r=dao
https://hg.mozilla.org/mozilla-central/rev/dc62d0350cbb
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Please request Beta approval on this when you get a chance.
Flags: qe-verify+
Flags: needinfo?(mstriemer)
Attached image Bug1477591.gif
I was able to reproduce this issue on Firefox 62.0b15 (20180806191531) under Win 7 64-bit and Mac OS X 10.13.3.

This issue is verified as fixed on Firefox 63.0a1(20180807220134) under Win 7 64-bit and Mac OS X 10.13.3.

Please see the attached video.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Dão, can you request uplift if you think this fix should go into 62? If it goes too much later it'll be too late in the beta cycle.
Flags: needinfo?(dao+bmo)
Comment on attachment 8996058 [details]
Bug 1477591 - Use default favicon in all tabs menu

Approval Request Comment
[Feature/Bug causing the regression]: bug 1446101
[User impact if declined]: default favicon missing in all tabs panel
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: trivial CSS fix
[String changes made/needed]: /
Flags: needinfo?(mstriemer)
Flags: needinfo?(dao+bmo)
Attachment #8996058 - Flags: approval-mozilla-beta?
Comment on attachment 8996058 [details]
Bug 1477591 - Use default favicon in all tabs menu

Minor CSS fix for all tabs panel, verified in nightly; let's uplift for beta 18.
Attachment #8996058 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified on 62.0b18 Build ID: 20180816151750
with Windows 10 x64, Mac OS X 10.11 and Ubuntu 18.10 x64.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.