Skip to content

Revert the change to remove font-family from 'html'#688

Closed
yuheiy wants to merge 1 commit into
necolas:masterfrom
yuheiy:revert-opinionated-chagnes
Closed

Revert the change to remove font-family from 'html'#688
yuheiy wants to merge 1 commit into
necolas:masterfrom
yuheiy:revert-opinionated-chagnes

Conversation

@yuheiy
Copy link
Copy Markdown
Contributor

@yuheiy yuheiy commented Jun 11, 2017 •
edited
Loading

From the README:

Corrects bugs and common browser inconsistencies.

Should not we also revert this change?

@garrettw
Copy link
Copy Markdown

garrettw commented Jun 11, 2017

But what is the most common default for that property? Probably not sans-serif, I would guess.
If it were to be added back in, I'd think it should merely reflect the most common default.

@yuheiy
Copy link
Copy Markdown
Contributor Author

yuheiy commented Jun 12, 2017

I think it is sans-serif. These are computed values for each browser.

Chrome: sans-serif
Firefox: sans-serif
Safari: -webkit-standard
IE: Times New Roman

And many websites use sans-serif.

body { margin: 0 } is not the common default of the browsers. But it is declared to remove styles that are not useful.

@jonathantneal
Copy link
Copy Markdown
Contributor

jonathantneal commented Jun 12, 2017

@yuheiy, 1. how are you determining the most widely used font on the web, and 2. would you please provide the test you are using to determine those computed values?

@garrettw
Copy link
Copy Markdown

garrettw commented Jun 12, 2017 •
edited
Loading

For the record, my personal installation of Firefox has never had its default font changed in Settings, and it shows a value of Times New Roman.
Also, my installation of Chrome has also not been changed, and its Standard Font is also set to Times New Roman.

To further test this, I navigated to about:blank in both browsers, and after inspecting the DOM of the blank page, I found that font-family was set to serif in Firefox, but in Chrome I had trouble locating the relevant user agent style. The best I could find was, when I inspected the <html> and <body> tags, went to the Properties tab, expanded the top bullet (the tag name), and expanded the style attribute, I found this on both: fontFamily: ""

@yuheiy
Copy link
Copy Markdown
Contributor Author

yuheiy commented Jun 12, 2017

  1. Sorry, this opinion was not grounded and it was based on my experience.
  2. getComputedStyle(document.documentElement).fontFamily on MacOS. But my language setting is Japanese. Is serif selected in English?

@Toddses
Copy link
Copy Markdown

Toddses commented Jun 12, 2017

In English getComputedStyle(document.documentElement).fontFamily is "Times" (at least in Chrome).

@garrettw
Copy link
Copy Markdown

garrettw commented Jun 12, 2017

I just checked on my Windows version of Chrome by navigating to about:blank followed by javascript:alert(getComputedStyle(document.documentElement).fontFamily); and the popup said "Times New Roman".

@simonsmith
Copy link
Copy Markdown

simonsmith commented Jun 12, 2017

Isn't this PR more about reverting to what was there pre 6.0.0 rather than what browsers use as a default? Seeing as the other changes were reverted I can see the sense in this change for consistency sake.

@jonathantneal
Copy link
Copy Markdown
Contributor

jonathantneal commented Jun 12, 2017

@simonsmith, that’s a valid reason for restoring it, but folks are probably chiming in because the original comment left it ambiguous:

Corrects bugs and common browser inconsistencies

And, at the time, sans-serif was an opinionated change; neither an inconsistency or a bug. However, @yuheiy is saying their Japanese Mac desktop defaults to sans-serif. That seems worth investigating.

But back to your point, @simonsmith; @necolas, if you add this back in another release for consistency sake, that leaves any research moot. To that end, I couldn’t justify the time to look into this, unless I knew the project was actually inspecting this PR as a browser inconsistency issue.

@yuheiy
Copy link
Copy Markdown
Contributor Author

yuheiy commented Jun 13, 2017 •
edited
Loading

Sorry, my comment was not accurate.
I navigated major browsers to about:blank and execute getComputedStyle(document.documentElement).fontFamily. Then, results are as follows.

Chrome: "Hiragino Kaku Gothic ProN"
Firefox: "sans-serif"
Safari: "-webkit-standard"

"Hiragino Kaku Gothic ProN" is Japanese sans-serif.

@jonathantneal
Copy link
Copy Markdown
Contributor

jonathantneal commented Jan 30, 2018

Thanks for that update, @yuheiy. It seems then like this PR should be closed. Can you close it on your end?

@yuheiy
Copy link
Copy Markdown
Contributor Author

yuheiy commented Jan 30, 2018

Bye

@yuheiy yuheiy closed this Jan 30, 2018
@yuheiy yuheiy deleted the revert-opinionated-chagnes branch January 30, 2018 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yuheiy @garrettw @jonathantneal @Toddses @simonsmith