New network error pages, part 1.

New network error pages, part 1.

Significantly changes the layout of the network error pages,
and makes the summary/suggestions pane hidden by default.

BUG=174194
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=191712

[mmenke, 2013-02-22 20:12:31]

Sample pages:
Desktop:  https://www.corp.google.com/~mmenke/error.htm
Mobile:  https://www.corp.google.com/~mmenke/error-mobile.htm
(Mobile may not exactly match set 7, but it's close)

I'm not using buttons quite as large for mobile as in the mocks because I'm
concerned about internationalization (Reload / more / less may be larger in some
languages) and other devices.  I've tested the HTML on the Nexus S, Galaxy
Nexus, Nexus 7, and iPhone.

Open to suggestions on how better to better size the buttons on different
devices.

Once this is landed, I'll reformat the suggestions and add the images in
separate CLs.

[mmenke, 2013-02-25 18:33:46]

[+eroman]:  Mind reviewing this?  I'll be doing some changes to the text and
suggestions in followup CLs, and figure it's a good idea to have another network
guy on board for these CLs.

[mmenke, 2013-02-25 18:38:51]

https://chromiumcodereview.appspot.com/12277011/diff/47003/chrome/common/loca...
File chrome/common/localized_error.cc (right):

https://chromiumcodereview.appspot.com/12277011/diff/47003/chrome/common/loca...
chrome/common/localized_error.cc:411: IDS_ERRORPAGES_DETAILS_NULL,
Admittedly, getting rid of this isn't part of the mocks, but I always feel that
the "Unknown Error" would be better text for
ERR_WE_HAVE_NO_IDEA_WHAT_WE_ARE_DOING.

[mmenke, 2013-02-25 20:27:32]

Forgot to link the mocks:

desktop: 
https://www.corp.google.com/~kenmoore/mocks/chrome/ErrorPages/Pass4/chrome-er...
mobile: 
https://docs.google.com/a/google.com/folder/d/0B21JpVYxVLsAUmktYkJLRXk5S00/edit

[newt (away), 2013-02-25 23:07:52]

Looks generally good, thanks!

I have a couple small comments inline, and two design spec questions:
 1. Are you going to add images at the top of the page?
 2. Are you going to change the display of the suggestions list to match the
mocks? i.e. use paragraphs with bold header sentences instead of using bullet
points

https://codereview.chromium.org/12277011/diff/47003/chrome/common/localized_e...
File chrome/common/localized_error.cc (right):

https://codereview.chromium.org/12277011/diff/47003/chrome/common/localized_e...
chrome/common/localized_error.cc:485: error_string =
base::IntToString16(error_code);
This seems fine to me to change to display of HTTP error codes from words to
numbers.

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:14: <if expr="not pp_ifdef('android')
and not pp_ifdef('ios')">
you can also use if-then-else syntax here:

<if expr="...">
  <then>
    margin: ...;
  </then>
  <else>
    margin: ...;
  </else>
</if>

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:141: <if expr="not pp_ifdef('android')
and not pp_ifdef('ios')">
I think it'd be more readable to have a single if-then-else clause here and just
alphabetize the properties within the if-then-else clause.

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:204:
document.getElementById('lessButton').style.display = 'inline';
rather than using a separate button for "less" and "more", could you just change
the text on a single button?

You can use jsvalues to store the translations of "more" and "less":
https://code.google.com/p/google-jstemplate/wiki/TemplateProcessingInstructio...

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:282: <li jsselect="suggestionsReload">
this <li> should be removed, right?

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:319: <div jsselect="errorDetails"
jscontent="$this"></div>
I believe this could just be jscontent="errorDetails" (without jsselect
altogether). Same for the more/less buttons above.

[mmenke, 2013-02-25 23:18:07]

On 2013/02/25 23:07:52, newt wrote:
> Looks generally good, thanks!
> 
> I have a couple small comments inline, and two design spec questions:
>  1. Are you going to add images at the top of the page?
>  2. Are you going to change the display of the suggestions list to match the
> mocks? i.e. use paragraphs with bold header sentences instead of using bullet
> points

I'll respond to the rest tomorrow, but just to respond to these questions - yes
and yes.  I'm splitting it up into a couple CLs so they're easier to review.

I expect 5 CLs, including this one.  The other 4:

* Add images and the iframe page (Was going to use a separate file for it, but
I'm back to thinking one file for both is simpler)
* Update suggestion format (They're going from one line of text to heading +
text, so that CL will be a bunch of string changes.  Also plan to use a list of
all suggestions rather than a dictionary).
* Update some of the other text (Shorter headings and descriptions in some
cases).  Pure string change.
* Get rid of the separate function for posts + ERR_CACHE_MISS.  Instead, add a
bool identifying posts the the LocalizerError function, and hide the reload
button for them (And add a suggestion to reload for those cases only).  Reload
button doesn't work well for them.

Hope to get the next two submitted for review this week.

[mmenke, 2013-02-26 03:46:59]

Thanks for the feedback!

https://codereview.chromium.org/12277011/diff/47003/chrome/common/localized_e...
File chrome/common/localized_error.cc (right):

https://codereview.chromium.org/12277011/diff/47003/chrome/common/localized_e...
chrome/common/localized_error.cc:485: error_string =
base::IntToString16(error_code);
On 2013/02/25 23:07:52, newt wrote:
> This seems fine to me to change to display of HTTP error codes from words to
> numbers.

This change was in the mocks, and I think it's a good idea - the words weren't
internationalized, since there are part of the "error code for chrome devs"
section, but displaying more non-internationalized text than strictly necessary
isn't good.

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:14: <if expr="not pp_ifdef('android')
and not pp_ifdef('ios')">
On 2013/02/25 23:07:52, newt wrote:
> you can also use if-then-else syntax here:
> 
> <if expr="...">
>   <then>
>     margin: ...;
>   </then>
>   <else>
>     margin: ...;
>   </else>
> </if>

This does not seem to work.  I always get either everything (including the then
/ elses) in the HTML, or neither, depending on the expression.

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:141: <if expr="not pp_ifdef('android')
and not pp_ifdef('ios')">
On 2013/02/25 23:07:52, newt wrote:
> I think it'd be more readable to have a single if-then-else clause here and
just
> alphabetize the properties within the if-then-else clause.

Merged the if's.  I've also moved all <if>s to the end of the relevant CSS
blocks, so there's a consistent rule.

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:204:
document.getElementById('lessButton').style.display = 'inline';
On 2013/02/25 23:07:52, newt wrote:
> rather than using a separate button for "less" and "more", could you just
change
> the text on a single button?
> 
> You can use jsvalues to store the translations of "more" and "less":
>
https://code.google.com/p/google-jstemplate/wiki/TemplateProcessingInstructio...

Done.  Does make things a bit simpler, although now I have to toggle the
displayed value calculating max button width.

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:282: <li jsselect="suggestionsReload">
On 2013/02/25 23:07:52, newt wrote:
> this <li> should be removed, right?

Done.

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:319: <div jsselect="errorDetails"
jscontent="$this"></div>
On 2013/02/25 23:07:52, newt wrote:
> I believe this could just be jscontent="errorDetails" (without jsselect
> altogether). Same for the more/less buttons above.

Thanks!  Done.

[newt (away), 2013-02-27 02:04:14]

lgtm

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:14: <if expr="not pp_ifdef('android')
and not pp_ifdef('ios')">
Odd. This syntax should work: 

https://codereview.chromium.org/11155024/diff/9008/grit/node/misc_unittest.py

but it really doesn't matter.


On 2013/02/26 03:47:00, mmenke wrote:
> On 2013/02/25 23:07:52, newt wrote:
> > you can also use if-then-else syntax here:
> > 
> > <if expr="...">
> >   <then>
> >     margin: ...;
> >   </then>
> >   <else>
> >     margin: ...;
> >   </else>
> > </if>
> 
> This does not seem to work.  I always get either everything (including the
then
> / elses) in the HTML, or neither, depending on the expression.

[mmenke, 2013-02-27 02:11:01]

Thanks!

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/47003/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:14: <if expr="not pp_ifdef('android')
and not pp_ifdef('ios')">
On 2013/02/27 02:04:14, newt wrote:
> Odd. This syntax should work: 
> 
> https://codereview.chromium.org/11155024/diff/9008/grit/node/misc_unittest.py
> 
> but it really doesn't matter.
> 
> 
> On 2013/02/26 03:47:00, mmenke wrote:
> > On 2013/02/25 23:07:52, newt wrote:
> > > you can also use if-then-else syntax here:
> > > 
> > > <if expr="...">
> > >   <then>
> > >     margin: ...;
> > >   </then>
> > >   <else>
> > >     margin: ...;
> > >   </else>
> > > </if>
> > 
> > This does not seem to work.  I always get either everything (including the
> then
> > / elses) in the HTML, or neither, depending on the expression.
> 

Yea, I saw that - no idea why it wasn't work.  Wouldn't think that we'd have two
separate parsers for <if>'s, but I can't think of any other explanation.  I did
spend some time playing with it.  Even did a sync, just in case it was a really
new feature, but no luck.

[eroman, 2013-02-27 02:26:16]

lgtm

https://codereview.chromium.org/12277011/diff/45004/chrome/common/localized_e...
File chrome/common/localized_error.cc (right):

https://codereview.chromium.org/12277011/diff/45004/chrome/common/localized_e...
chrome/common/localized_error.cc:438: // If there are any suggestions other than
reload, popuplate the suggestion
typo: populate

https://codereview.chromium.org/12277011/diff/45004/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/45004/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:59: <if expr="pp_ifdef('android') or
pp_ifdef('ios')">
Can this be an "else"? (same question applies to rest of file).

[mmenke, 2013-02-27 02:43:01]

Thanks!

https://chromiumcodereview.appspot.com/12277011/diff/45004/chrome/common/loca...
File chrome/common/localized_error.cc (right):

https://chromiumcodereview.appspot.com/12277011/diff/45004/chrome/common/loca...
chrome/common/localized_error.cc:438: // If there are any suggestions other than
reload, popuplate the suggestion
On 2013/02/27 02:26:16, eroman wrote:
> typo: populate

Done.

https://chromiumcodereview.appspot.com/12277011/diff/45004/chrome/renderer/re...
File chrome/renderer/resources/neterror.html (right):

https://chromiumcodereview.appspot.com/12277011/diff/45004/chrome/renderer/re...
chrome/renderer/resources/neterror.html:59: <if expr="pp_ifdef('android') or
pp_ifdef('ios')">
On 2013/02/27 02:26:16, eroman wrote:
> Can this be an "else"? (same question applies to rest of file).

Unfortunately, while there is an else syntax, I can't seem to get it working. 
Perhaps it doesn't for processed HTML files?

[mmenke, 2013-02-27 02:55:37]

[+thakis]:  For OWNERs review.

[mmenke, 2013-03-01 20:50:03]

On 2013/02/27 02:55:37, mmenke wrote:
> [+thakis]:  For OWNERs review.

thakis:  Ping!

[Nico, 2013-03-04 08:22:35]

Sorry, I missed this. Feel free to ping me a lot earlier (after 4h or so).

Generally, can you try doing responsive web design stuff for the html instead of
basing this on android or iOS? (I didn't leave comments on all the ifs.)

https://chromiumcodereview.appspot.com/12277011/diff/54001/chrome/renderer/re...
File chrome/renderer/resources/neterror.html (right):

https://chromiumcodereview.appspot.com/12277011/diff/54001/chrome/renderer/re...
chrome/renderer/resources/neterror.html:19: margin: 15px;
Should this be done via a media query instead? Seems like this would make sense
on desktop too if the window is very narrow.

https://chromiumcodereview.appspot.com/12277011/diff/54001/chrome/renderer/re...
chrome/renderer/resources/neterror.html:26: <if expr="not pp_ifdef('android')
and not pp_ifdef('ios')">
likewise

https://chromiumcodereview.appspot.com/12277011/diff/54001/chrome/renderer/re...
chrome/renderer/resources/neterror.html:38: /* Not done on mobile for
performance reasons. */
(this is ok)

[mmenke, 2013-03-04 21:24:11]

Thanks for the feedback!

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:19: margin: 15px;
On 2013/03/04 08:22:35, Nico wrote:
> Should this be done via a media query instead? Seems like this would make
sense
> on desktop too if the window is very narrow.

Done.

The situation for font sizes is kinda weird.  There seems to be no functional
way to get DPI.  There is "-webkit-min-device-pixel-ratio", which gives device
pixels per logical pixel, but since we specify sizes in logical pixels, that's
pretty pointless for this purpose.  What we really need is a way to get some
hint as to the physical size of a logical pixel.

I've just gone with using bigger fonts on touch devices, which would miss small
non-touch displays and uses large fonts on ChromeBook Pixels.  I think this is
good enough (I've tested it on a Pixel), but I'd love to have a better solution.

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:26: <if expr="not pp_ifdef('android')
and not pp_ifdef('ios')">
On 2013/03/04 08:22:35, Nico wrote:
> likewise

Done.

[mmenke, 2013-03-04 21:24:58]

On 2013/03/04 21:24:11, mmenke wrote:
> Thanks for the feedback!
> 
>
https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
> File chrome/renderer/resources/neterror.html (right):
> 
>
https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
> chrome/renderer/resources/neterror.html:19: margin: 15px;
> On 2013/03/04 08:22:35, Nico wrote:
> > Should this be done via a media query instead? Seems like this would make
> sense
> > on desktop too if the window is very narrow.
> 
> Done.
> 
> The situation for font sizes is kinda weird.  There seems to be no functional
> way to get DPI.  There is "-webkit-min-device-pixel-ratio", which gives device
> pixels per logical pixel, but since we specify sizes in logical pixels, that's
> pretty pointless for this purpose.  What we really need is a way to get some
> hint as to the physical size of a logical pixel.
> 
> I've just gone with using bigger fonts on touch devices, which would miss
small
> non-touch displays and uses large fonts on ChromeBook Pixels.  I think this is
> good enough (I've tested it on a Pixel), but I'd love to have a better
solution.
> 
>
https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
> chrome/renderer/resources/neterror.html:26: <if expr="not pp_ifdef('android')
> and not pp_ifdef('ios')">
> On 2013/03/04 08:22:35, Nico wrote:
> > likewise
> 
> Done.

Oh, and I've updated the sample page -
https://www.corp.google.com/~mmenke/error.htm has the new template (desktop and
mobile), if anyone's interested.

[Nico, 2013-03-04 21:28:49]

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:19: margin: 15px;
On 2013/03/04 21:24:11, mmenke wrote:
> On 2013/03/04 08:22:35, Nico wrote:
> > Should this be done via a media query instead? Seems like this would make
> sense
> > on desktop too if the window is very narrow.
> 
> Done.
> 
> The situation for font sizes is kinda weird.  There seems to be no functional
> way to get DPI.  There is "-webkit-min-device-pixel-ratio", which gives device
> pixels per logical pixel, but since we specify sizes in logical pixels, that's
> pretty pointless for this purpose.  What we really need is a way to get some
> hint as to the physical size of a logical pixel.

Note that 1 css px isn't a physical pixel, but a "css pixel" which automatically
takes dpiness into account. A 13px font is ~13px high on lodpi devices, 26px on
retina, etc.

Touch devices give pages a viewport if the page doesn't explicitly set one, and
it'll do things like font scaling too. You can set an explicit viewport to work
around this ( https://developer.mozilla.org/en-US/docs/Mobile/Viewport_meta_tag
is an ok intro).
> 
> I've just gone with using bigger fonts on touch devices, which would miss
small
> non-touch displays and uses large fonts on ChromeBook Pixels.  I think this is
> good enough (I've tested it on a Pixel), but I'd love to have a better
solution.

[mmenke, 2013-03-04 23:10:41]

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:19: margin: 15px;
On 2013/03/04 21:28:50, Nico wrote:
> On 2013/03/04 21:24:11, mmenke wrote:
> > On 2013/03/04 08:22:35, Nico wrote:
> > > Should this be done via a media query instead? Seems like this would make
> > sense
> > > on desktop too if the window is very narrow.
> > 
> > Done.
> > 
> > The situation for font sizes is kinda weird.  There seems to be no
functional
> > way to get DPI.  There is "-webkit-min-device-pixel-ratio", which gives
device
> > pixels per logical pixel, but since we specify sizes in logical pixels,
that's
> > pretty pointless for this purpose.  What we really need is a way to get some
> > hint as to the physical size of a logical pixel.
> 
> Note that 1 css px isn't a physical pixel, but a "css pixel" which
automatically
> takes dpiness into account. A 13px font is ~13px high on lodpi devices, 26px
on
> retina, etc.
> 
> Touch devices give pages a viewport if the page doesn't explicitly set one,
and
> it'll do things like font scaling too. You can set an explicit viewport to
work
> around this (
https://developer.mozilla.org/en-US/docs/Mobile/Viewport_meta_tag
> is an ok intro).
> > 
> > I've just gone with using bigger fonts on touch devices, which would miss
> small
> > non-touch displays and uses large fonts on ChromeBook Pixels.  I think this
is
> > good enough (I've tested it on a Pixel), but I'd love to have a better
> solution.
> 

I don't see anything actionable in this comment.

This page already uses <meta name="viewport" content="width=device-width" />. 
The problem is that fonts legible on desktop are too small on mobile.  So we
either need to:

1)  Explicitly use larger fonts on mobile or when CSS DPI is high.
2)  Make CSS pixels larger on mobile.

There's no CSS rule for 1) that I can find, and you don't want me to use <if>'s.

There also doesn't seem to be any reasonable way to do 2).  "initial-scale" is
basically like zooming a canvas image in, which we don't want to do - we want a
100% width to mean 100% of the screen. not 100% of the screen before we zoomed
in.

width=constant results in increasing the size of everything, fonts included, in
proportion to device size.

We actually want constant font size across mobile devices.

[mmenke, 2013-03-05 15:02:49]

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:19: margin: 15px;
On 2013/03/04 23:10:41, mmenke wrote:
> On 2013/03/04 21:28:50, Nico wrote:
> > On 2013/03/04 21:24:11, mmenke wrote:
> > > On 2013/03/04 08:22:35, Nico wrote:
> > > > Should this be done via a media query instead? Seems like this would
make
> > > sense
> > > > on desktop too if the window is very narrow.
> > > 
> > > Done.
> > > 
> > > The situation for font sizes is kinda weird.  There seems to be no
> functional
> > > way to get DPI.  There is "-webkit-min-device-pixel-ratio", which gives
> device
> > > pixels per logical pixel, but since we specify sizes in logical pixels,
> that's
> > > pretty pointless for this purpose.  What we really need is a way to get
some
> > > hint as to the physical size of a logical pixel.
> > 
> > Note that 1 css px isn't a physical pixel, but a "css pixel" which
> automatically
> > takes dpiness into account. A 13px font is ~13px high on lodpi devices, 26px
> on
> > retina, etc.
> > 
> > Touch devices give pages a viewport if the page doesn't explicitly set one,
> and
> > it'll do things like font scaling too. You can set an explicit viewport to
> work
> > around this (
> https://developer.mozilla.org/en-US/docs/Mobile/Viewport_meta_tag
> > is an ok intro).
> > > 
> > > I've just gone with using bigger fonts on touch devices, which would miss
> > small
> > > non-touch displays and uses large fonts on ChromeBook Pixels.  I think
this
> is
> > > good enough (I've tested it on a Pixel), but I'd love to have a better
> > solution.
> > 
> 
> I don't see anything actionable in this comment.
> 
> This page already uses <meta name="viewport" content="width=device-width" />. 
> The problem is that fonts legible on desktop are too small on mobile.  So we
> either need to:
> 
> 1)  Explicitly use larger fonts on mobile or when CSS DPI is high.
> 2)  Make CSS pixels larger on mobile.
> 
> There's no CSS rule for 1) that I can find, and you don't want me to use
<if>'s.
> 
> There also doesn't seem to be any reasonable way to do 2).  "initial-scale" is
> basically like zooming a canvas image in, which we don't want to do - we want
a
> 100% width to mean 100% of the screen. not 100% of the screen before we zoomed
> in.
> 
> width=constant results in increasing the size of everything, fonts included,
in
> proportion to device size.
> 
> We actually want constant font size across mobile devices.

I've read it again, and still see nothing useful here.

So...What's the path forward on this CL?

[Nico, 2013-03-05 15:06:23]

(Sorry, I likely won't be able to look at again this before Thu)

On Tue, Mar 5, 2013 at 4:02 PM,  <mmenke@chromium.org> wrote:
>
>
https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
> File chrome/renderer/resources/neterror.html (right):
>
>
https://codereview.chromium.org/12277011/diff/54001/chrome/renderer/resources...
> chrome/renderer/resources/neterror.html:19: margin: 15px;
> On 2013/03/04 23:10:41, mmenke wrote:
>>
>> On 2013/03/04 21:28:50, Nico wrote:
>> > On 2013/03/04 21:24:11, mmenke wrote:
>> > > On 2013/03/04 08:22:35, Nico wrote:
>> > > > Should this be done via a media query instead? Seems like this
>
> would make
>>
>> > > sense
>> > > > on desktop too if the window is very narrow.
>> > >
>> > > Done.
>> > >
>> > > The situation for font sizes is kinda weird.  There seems to be no
>> functional
>> > > way to get DPI.  There is "-webkit-min-device-pixel-ratio", which
>
> gives
>>
>> device
>> > > pixels per logical pixel, but since we specify sizes in logical
>
> pixels,
>>
>> that's
>> > > pretty pointless for this purpose.  What we really need is a way
>
> to get some
>>
>> > > hint as to the physical size of a logical pixel.
>> >
>> > Note that 1 css px isn't a physical pixel, but a "css pixel" which
>> automatically
>> > takes dpiness into account. A 13px font is ~13px high on lodpi
>
> devices, 26px
>>
>> on
>> > retina, etc.
>> >
>> > Touch devices give pages a viewport if the page doesn't explicitly
>
> set one,
>>
>> and
>> > it'll do things like font scaling too. You can set an explicit
>
> viewport to
>>
>> work
>> > around this (
>> https://developer.mozilla.org/en-US/docs/Mobile/Viewport_meta_tag
>> > is an ok intro).
>> > >
>> > > I've just gone with using bigger fonts on touch devices, which
>
> would miss
>>
>> > small
>> > > non-touch displays and uses large fonts on ChromeBook Pixels.  I
>
> think this
>>
>> is
>> > > good enough (I've tested it on a Pixel), but I'd love to have a
>
> better
>>
>> > solution.
>> >
>
>
>> I don't see anything actionable in this comment.
>
>
>> This page already uses <meta name="viewport"
>
> content="width=device-width" />.
>>
>> The problem is that fonts legible on desktop are too small on mobile.
>
> So we
>>
>> either need to:
>
>
>> 1)  Explicitly use larger fonts on mobile or when CSS DPI is high.
>> 2)  Make CSS pixels larger on mobile.
>
>
>> There's no CSS rule for 1) that I can find, and you don't want me to
>
> use <if>'s.
>
>> There also doesn't seem to be any reasonable way to do 2).
>
> "initial-scale" is
>>
>> basically like zooming a canvas image in, which we don't want to do -
>
> we want a
>>
>> 100% width to mean 100% of the screen. not 100% of the screen before
>
> we zoomed
>>
>> in.
>
>
>> width=constant results in increasing the size of everything, fonts
>
> included, in
>>
>> proportion to device size.
>
>
>> We actually want constant font size across mobile devices.
>
>
> I've read it again, and still see nothing useful here.
>
> So...What's the path forward on this CL?
>
> https://codereview.chromium.org/12277011/

[mmenke, 2013-03-05 15:12:33]

On 2013/03/05 15:06:23, Nico wrote:
> (Sorry, I likely won't be able to look at again this before Thu)

Thanks for the update.  Punting the new error pages to M28 (Since this is only
the first CL, and I want to see the full thing get some use before branch), so
there's no rush on this.  Won't be landing for a couple weeks, anyways.

[Nico, 2013-03-12 00:03:55]

Sorry, forgot about this.

Maybe johnme has a good suggestion on what to do about the font size. John: Text
on this webui page is too small on mobile devices, so there are now a few
compile-time dependend font settings in this CL. This feels wrong to me. Do you
have a better idea?

(Worst case, add a

    @media only screen and (min-width: 641px) {
        font-size: 125%;
    }

I guess instead of the <if>?)

https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:133: padding: 10px 20px;
Can you pull the padding out of the if?

[mmenke, 2013-03-12 00:09:34]

On 2013/03/12 00:03:55, Nico wrote:
> Sorry, forgot about this.
> 
> Maybe johnme has a good suggestion on what to do about the font size. John:
Text
> on this webui page is too small on mobile devices, so there are now a few
> compile-time dependend font settings in this CL. This feels wrong to me. Do
you
> have a better idea?
> 
> (Worst case, add a
> 
>     @media only screen and (min-width: 641px) {
>         font-size: 125%;
>     }
> 
> I guess instead of the <if>?)

I don't think we should not make the UI materially worse on small desktop
windows in pursuit of avoiding compile-time font choices.  This would also be
much too small on larger mobile screens, like the Nexus 10.

https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources...
chrome/renderer/resources/neterror.html:133: padding: 10px 20px;
On 2013/03/12 00:03:55, Nico wrote:
> Can you pull the padding out of the if?

The buttons on the mocks are significantly larger on Android than on desktop.

[johnme, 2013-03-12 13:41:54]

Can you give more context?

I see that there's a width=device-width viewport, which is good (though it
seems redundant to ifdef it so the viewport is only on Android and iOS).
Given that, any font sizes specified in px or pt (though pt is a strange
unit to use on the web) should appear at approximately the same perceived
size on Android and iOS as on desktop. For example a 320ppi phone will get
a devicePixelRatio of 2x, and hence a 16px font will be rendered at 32
physical screen pixels size. Whereas a 160ppi phone will get a
devicePixelRatio of 1x, and hence a 16px font will be rendered at 16
physical screen pixels size. Meanwhile a 96ppi desktop will also get a
devicePixelRatio of 1x, and so a 16px font will be rendered at 16 physical
screen pixels size. And a 239ppi Chromebook Pixel will get a
devicePixelRatio of 2x, and so a 16px font will be rendered at 32 physical
screen pixels size.

Now, you may notice that the devicePixelRatio values aren't proportional to
the PPIs. That's because the devicePixelRatio is based on the definition of
CSS pixels <http://dev.w3.org/csswg/css3-values/#reference-pixel>, which
(sensibly) takes into account the distance at which a device is viewed. In
particular, a phone is usually viewed from a short distance (hence the base
1x PPI on phones is 160), while laptops are usually viewed from medium
distance (hence the base 1x PPI is somewhere around 120 on laptops), and
desktops are usually viewed from relatively far away (hence the base 1x PPI
on desktops is 96).

Hence, the perceived sizes (and legibility) taking into account viewing
distance should already be very similar on all devices without the web
developer having to do anything (apart from setting that width=device-width
viewport).

Tablets are somewhere between phones and laptops, though you'll find quite
a few manufacturers incorrectly assuming they are viewed from the same
distance as phones, and hence setting Android's
DisplayMetrics.density<http://developer.android.com/reference/android/util/DisplayMetrics.html#density>
(which
determines devicePixelRatio) too low. Chrome for Android compensates for
this by making text up to 30% bigger on tablets, though the exact
implementation details of that are subject to
change<https://code.google.com/p/chromium/issues/detail?id=135869>
.

Does that clear things up at all (or did I just take you further down the
rabbit hole)?
--John


On Tue, Mar 12, 2013 at 12:09 AM, <mmenke@chromium.org> wrote:

> On 2013/03/12 00:03:55, Nico wrote:
>
>> Sorry, forgot about this.
>>
>
>  Maybe johnme has a good suggestion on what to do about the font size.
>> John:
>>
> Text
>
>> on this webui page is too small on mobile devices, so there are now a few
>> compile-time dependend font settings in this CL. This feels wrong to me.
>> Do
>>
> you
>
>> have a better idea?
>>
>
>  (Worst case, add a
>>
>
>       @media only screen and (min-width: 641px) {
>>          font-size: 125%;
>>      }
>>
>
>  I guess instead of the <if>?)
>>
>
> I don't think we should not make the UI materially worse on small desktop
> windows in pursuit of avoiding compile-time font choices.  This would also
> be
> much too small on larger mobile screens, like the Nexus 10.
>
>
>
> https://codereview.chromium.**org/12277011/diff/81001/**
>
chrome/renderer/resources/**neterror.html<https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources/neterror.html>
> File chrome/renderer/resources/**neterror.html (right):
>
> https://codereview.chromium.**org/12277011/diff/81001/**
>
chrome/renderer/resources/**neterror.html#newcode133<https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources/neterror.html#newcode133>
> chrome/renderer/resources/**neterror.html:133: padding: 10px 20px;
> On 2013/03/12 00:03:55, Nico wrote:
>
>> Can you pull the padding out of the if?
>>
>
> The buttons on the mocks are significantly larger on Android than on
> desktop.
>
>
https://codereview.chromium.**org/12277011/<https://codereview.chromium.org/1...
>

[mmenke, 2013-03-12 13:49:45]

I'm adhering to mocks that I did not create.  The font sizes on Android/iOS had
larger CSS "px" sizes than on desktop.  Mobile folks created the mobile mocks.

Moreover, viewing the error page with the "same" "px" size on both mobile and
desktop, the decision makes sense.  The error pages are near illegible with the
"same" size on mobile as on desktop, and the text on desktop seems abnormally
large when using the mobile font size.

The mobile mocks also have a lot more spacing around the buttons, because a
button you touch generally needs to be larger than one you click on.

On 2013/03/12 13:41:54, johnme wrote:
> Can you give more context?
> 
> I see that there's a width=device-width viewport, which is good (though it
> seems redundant to ifdef it so the viewport is only on Android and iOS).
> Given that, any font sizes specified in px or pt (though pt is a strange
> unit to use on the web) should appear at approximately the same perceived
> size on Android and iOS as on desktop. For example a 320ppi phone will get
> a devicePixelRatio of 2x, and hence a 16px font will be rendered at 32
> physical screen pixels size. Whereas a 160ppi phone will get a
> devicePixelRatio of 1x, and hence a 16px font will be rendered at 16
> physical screen pixels size. Meanwhile a 96ppi desktop will also get a
> devicePixelRatio of 1x, and so a 16px font will be rendered at 16 physical
> screen pixels size. And a 239ppi Chromebook Pixel will get a
> devicePixelRatio of 2x, and so a 16px font will be rendered at 32 physical
> screen pixels size.
> 
> Now, you may notice that the devicePixelRatio values aren't proportional to
> the PPIs. That's because the devicePixelRatio is based on the definition of
> CSS pixels <http://dev.w3.org/csswg/css3-values/#reference-pixel>, which
> (sensibly) takes into account the distance at which a device is viewed. In
> particular, a phone is usually viewed from a short distance (hence the base
> 1x PPI on phones is 160), while laptops are usually viewed from medium
> distance (hence the base 1x PPI is somewhere around 120 on laptops), and
> desktops are usually viewed from relatively far away (hence the base 1x PPI
> on desktops is 96).
> 
> Hence, the perceived sizes (and legibility) taking into account viewing
> distance should already be very similar on all devices without the web
> developer having to do anything (apart from setting that width=device-width
> viewport).
> 
> Tablets are somewhere between phones and laptops, though you'll find quite
> a few manufacturers incorrectly assuming they are viewed from the same
> distance as phones, and hence setting Android's
>
DisplayMetrics.density<http://developer.android.com/reference/android/util/DisplayMetrics.html#density>
> (which
> determines devicePixelRatio) too low. Chrome for Android compensates for
> this by making text up to 30% bigger on tablets, though the exact
> implementation details of that are subject to
> change<https://code.google.com/p/chromium/issues/detail?id=135869>
> .
> 
> Does that clear things up at all (or did I just take you further down the
> rabbit hole)?
> --John
> 
> 
> On Tue, Mar 12, 2013 at 12:09 AM, <mailto:mmenke@chromium.org> wrote:
> 
> > On 2013/03/12 00:03:55, Nico wrote:
> >
> >> Sorry, forgot about this.
> >>
> >
> >  Maybe johnme has a good suggestion on what to do about the font size.
> >> John:
> >>
> > Text
> >
> >> on this webui page is too small on mobile devices, so there are now a few
> >> compile-time dependend font settings in this CL. This feels wrong to me.
> >> Do
> >>
> > you
> >
> >> have a better idea?
> >>
> >
> >  (Worst case, add a
> >>
> >
> >       @media only screen and (min-width: 641px) {
> >>          font-size: 125%;
> >>      }
> >>
> >
> >  I guess instead of the <if>?)
> >>
> >
> > I don't think we should not make the UI materially worse on small desktop
> > windows in pursuit of avoiding compile-time font choices.  This would also
> > be
> > much too small on larger mobile screens, like the Nexus 10.
> >
> >
> >
> > https://codereview.chromium.**org/12277011/diff/81001/**
> >
>
chrome/renderer/resources/**neterror.html<https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources/neterror.html>
> > File chrome/renderer/resources/**neterror.html (right):
> >
> > https://codereview.chromium.**org/12277011/diff/81001/**
> >
>
chrome/renderer/resources/**neterror.html#newcode133<https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources/neterror.html#newcode133>
> > chrome/renderer/resources/**neterror.html:133: padding: 10px 20px;
> > On 2013/03/12 00:03:55, Nico wrote:
> >
> >> Can you pull the padding out of the if?
> >>
> >
> > The buttons on the mocks are significantly larger on Android than on
> > desktop.
> >
> >
>
https://codereview.chromium.**org/12277011/%3Chttps://codereview.chromium.org...>
> >

[mmenke, 2013-03-12 13:51:40]

Mobile has larger buttons, rather.  Again, using the larger buttons on desktop
looks really weird when compared to, say, about:settings.

On 2013/03/12 13:49:45, mmenke wrote:
> I'm adhering to mocks that I did not create.  The font sizes on Android/iOS
had
> larger CSS "px" sizes than on desktop.  Mobile folks created the mobile mocks.
> 
> Moreover, viewing the error page with the "same" "px" size on both mobile and
> desktop, the decision makes sense.  The error pages are near illegible with
the
> "same" size on mobile as on desktop, and the text on desktop seems abnormally
> large when using the mobile font size.
> 
> The mobile mocks also have a lot more spacing around the buttons, because a
> button you touch generally needs to be larger than one you click on.
> 
> On 2013/03/12 13:41:54, johnme wrote:
> > Can you give more context?
> > 
> > I see that there's a width=device-width viewport, which is good (though it
> > seems redundant to ifdef it so the viewport is only on Android and iOS).
> > Given that, any font sizes specified in px or pt (though pt is a strange
> > unit to use on the web) should appear at approximately the same perceived
> > size on Android and iOS as on desktop. For example a 320ppi phone will get
> > a devicePixelRatio of 2x, and hence a 16px font will be rendered at 32
> > physical screen pixels size. Whereas a 160ppi phone will get a
> > devicePixelRatio of 1x, and hence a 16px font will be rendered at 16
> > physical screen pixels size. Meanwhile a 96ppi desktop will also get a
> > devicePixelRatio of 1x, and so a 16px font will be rendered at 16 physical
> > screen pixels size. And a 239ppi Chromebook Pixel will get a
> > devicePixelRatio of 2x, and so a 16px font will be rendered at 32 physical
> > screen pixels size.
> > 
> > Now, you may notice that the devicePixelRatio values aren't proportional to
> > the PPIs. That's because the devicePixelRatio is based on the definition of
> > CSS pixels <http://dev.w3.org/csswg/css3-values/#reference-pixel>, which
> > (sensibly) takes into account the distance at which a device is viewed. In
> > particular, a phone is usually viewed from a short distance (hence the base
> > 1x PPI on phones is 160), while laptops are usually viewed from medium
> > distance (hence the base 1x PPI is somewhere around 120 on laptops), and
> > desktops are usually viewed from relatively far away (hence the base 1x PPI
> > on desktops is 96).
> > 
> > Hence, the perceived sizes (and legibility) taking into account viewing
> > distance should already be very similar on all devices without the web
> > developer having to do anything (apart from setting that width=device-width
> > viewport).
> > 
> > Tablets are somewhere between phones and laptops, though you'll find quite
> > a few manufacturers incorrectly assuming they are viewed from the same
> > distance as phones, and hence setting Android's
> >
>
DisplayMetrics.density<http://developer.android.com/reference/android/util/DisplayMetrics.html#density>
> > (which
> > determines devicePixelRatio) too low. Chrome for Android compensates for
> > this by making text up to 30% bigger on tablets, though the exact
> > implementation details of that are subject to
> > change<https://code.google.com/p/chromium/issues/detail?id=135869>
> > .
> > 
> > Does that clear things up at all (or did I just take you further down the
> > rabbit hole)?
> > --John
> > 
> > 
> > On Tue, Mar 12, 2013 at 12:09 AM, <mailto:mmenke@chromium.org> wrote:
> > 
> > > On 2013/03/12 00:03:55, Nico wrote:
> > >
> > >> Sorry, forgot about this.
> > >>
> > >
> > >  Maybe johnme has a good suggestion on what to do about the font size.
> > >> John:
> > >>
> > > Text
> > >
> > >> on this webui page is too small on mobile devices, so there are now a few
> > >> compile-time dependend font settings in this CL. This feels wrong to me.
> > >> Do
> > >>
> > > you
> > >
> > >> have a better idea?
> > >>
> > >
> > >  (Worst case, add a
> > >>
> > >
> > >       @media only screen and (min-width: 641px) {
> > >>          font-size: 125%;
> > >>      }
> > >>
> > >
> > >  I guess instead of the <if>?)
> > >>
> > >
> > > I don't think we should not make the UI materially worse on small desktop
> > > windows in pursuit of avoiding compile-time font choices.  This would also
> > > be
> > > much too small on larger mobile screens, like the Nexus 10.
> > >
> > >
> > >
> > > https://codereview.chromium.**org/12277011/diff/81001/**
> > >
> >
>
chrome/renderer/resources/**neterror.html<https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources/neterror.html>
> > > File chrome/renderer/resources/**neterror.html (right):
> > >
> > > https://codereview.chromium.**org/12277011/diff/81001/**
> > >
> >
>
chrome/renderer/resources/**neterror.html#newcode133<https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources/neterror.html#newcode133>
> > > chrome/renderer/resources/**neterror.html:133: padding: 10px 20px;
> > > On 2013/03/12 00:03:55, Nico wrote:
> > >
> > >> Can you pull the padding out of the if?
> > >>
> > >
> > > The buttons on the mocks are significantly larger on Android than on
> > > desktop.
> > >
> > >
> >
>
https://codereview.chromium.**org/12277011/%253Chttps://codereview.chromium.o...>
> > >

[mmenke, 2013-03-12 14:41:24]

Go to https://www.corp.google.com/~mmenke/error.htm on desktop.  Notice that the
text in the page is about the same size of the text in the omnibox.

Now go to it on mobile.  Notice that the text in the page is a fair bit smaller
than the text in the omnibox.

Also try pressing the button on mobile.  It's not easy.

Note that the page is the desktop page, with the meta viewport line added.

On 2013/03/12 13:51:40, mmenke wrote:
> Mobile has larger buttons, rather.  Again, using the larger buttons on desktop
> looks really weird when compared to, say, about:settings.
> 
> On 2013/03/12 13:49:45, mmenke wrote:
> > I'm adhering to mocks that I did not create.  The font sizes on Android/iOS
> had
> > larger CSS "px" sizes than on desktop.  Mobile folks created the mobile
mocks.
> > 
> > Moreover, viewing the error page with the "same" "px" size on both mobile
and
> > desktop, the decision makes sense.  The error pages are near illegible with
> the
> > "same" size on mobile as on desktop, and the text on desktop seems
abnormally
> > large when using the mobile font size.
> > 
> > The mobile mocks also have a lot more spacing around the buttons, because a
> > button you touch generally needs to be larger than one you click on.
> > 
> > On 2013/03/12 13:41:54, johnme wrote:
> > > Can you give more context?
> > > 
> > > I see that there's a width=device-width viewport, which is good (though it
> > > seems redundant to ifdef it so the viewport is only on Android and iOS).
> > > Given that, any font sizes specified in px or pt (though pt is a strange
> > > unit to use on the web) should appear at approximately the same perceived
> > > size on Android and iOS as on desktop. For example a 320ppi phone will get
> > > a devicePixelRatio of 2x, and hence a 16px font will be rendered at 32
> > > physical screen pixels size. Whereas a 160ppi phone will get a
> > > devicePixelRatio of 1x, and hence a 16px font will be rendered at 16
> > > physical screen pixels size. Meanwhile a 96ppi desktop will also get a
> > > devicePixelRatio of 1x, and so a 16px font will be rendered at 16 physical
> > > screen pixels size. And a 239ppi Chromebook Pixel will get a
> > > devicePixelRatio of 2x, and so a 16px font will be rendered at 32 physical
> > > screen pixels size.
> > > 
> > > Now, you may notice that the devicePixelRatio values aren't proportional
to
> > > the PPIs. That's because the devicePixelRatio is based on the definition
of
> > > CSS pixels <http://dev.w3.org/csswg/css3-values/#reference-pixel>, which
> > > (sensibly) takes into account the distance at which a device is viewed. In
> > > particular, a phone is usually viewed from a short distance (hence the
base
> > > 1x PPI on phones is 160), while laptops are usually viewed from medium
> > > distance (hence the base 1x PPI is somewhere around 120 on laptops), and
> > > desktops are usually viewed from relatively far away (hence the base 1x
PPI
> > > on desktops is 96).
> > > 
> > > Hence, the perceived sizes (and legibility) taking into account viewing
> > > distance should already be very similar on all devices without the web
> > > developer having to do anything (apart from setting that
width=device-width
> > > viewport).
> > > 
> > > Tablets are somewhere between phones and laptops, though you'll find quite
> > > a few manufacturers incorrectly assuming they are viewed from the same
> > > distance as phones, and hence setting Android's
> > >
> >
>
DisplayMetrics.density<http://developer.android.com/reference/android/util/DisplayMetrics.html#density>
> > > (which
> > > determines devicePixelRatio) too low. Chrome for Android compensates for
> > > this by making text up to 30% bigger on tablets, though the exact
> > > implementation details of that are subject to
> > > change<https://code.google.com/p/chromium/issues/detail?id=135869>
> > > .
> > > 
> > > Does that clear things up at all (or did I just take you further down the
> > > rabbit hole)?
> > > --John
> > > 
> > > 
> > > On Tue, Mar 12, 2013 at 12:09 AM, <mailto:mmenke@chromium.org> wrote:
> > > 
> > > > On 2013/03/12 00:03:55, Nico wrote:
> > > >
> > > >> Sorry, forgot about this.
> > > >>
> > > >
> > > >  Maybe johnme has a good suggestion on what to do about the font size.
> > > >> John:
> > > >>
> > > > Text
> > > >
> > > >> on this webui page is too small on mobile devices, so there are now a
few
> > > >> compile-time dependend font settings in this CL. This feels wrong to
me.
> > > >> Do
> > > >>
> > > > you
> > > >
> > > >> have a better idea?
> > > >>
> > > >
> > > >  (Worst case, add a
> > > >>
> > > >
> > > >       @media only screen and (min-width: 641px) {
> > > >>          font-size: 125%;
> > > >>      }
> > > >>
> > > >
> > > >  I guess instead of the <if>?)
> > > >>
> > > >
> > > > I don't think we should not make the UI materially worse on small
desktop
> > > > windows in pursuit of avoiding compile-time font choices.  This would
also
> > > > be
> > > > much too small on larger mobile screens, like the Nexus 10.
> > > >
> > > >
> > > >
> > > > https://codereview.chromium.**org/12277011/diff/81001/**
> > > >
> > >
> >
>
chrome/renderer/resources/**neterror.html<https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources/neterror.html>
> > > > File chrome/renderer/resources/**neterror.html (right):
> > > >
> > > > https://codereview.chromium.**org/12277011/diff/81001/**
> > > >
> > >
> >
>
chrome/renderer/resources/**neterror.html#newcode133<https://codereview.chromium.org/12277011/diff/81001/chrome/renderer/resources/neterror.html#newcode133>
> > > > chrome/renderer/resources/**neterror.html:133: padding: 10px 20px;
> > > > On 2013/03/12 00:03:55, Nico wrote:
> > > >
> > > >> Can you pull the padding out of the if?
> > > >>
> > > >
> > > > The buttons on the mocks are significantly larger on Android than on
> > > > desktop.
> > > >
> > > >
> > >
> >
>
https://codereview.chromium.**org/12277011/%25253Chttps://codereview.chromium...>
> > > >

[johnme, 2013-03-12 14:42:39]

> I'm adhering to mocks that I did not create.  The font sizes on
Android/iOS had larger CSS "px" sizes than on desktop.

Were the designers definitely specifying sizes in CSS pixels not physical
screen pixels?

> The error pages are near illegible with the "same" size on mobile as on
desktop,
> and the text on desktop seems abnormally large when using the mobile font
size.

I could believe up to a ~30% difference, but "near illegible" seems very
strange. I took at look at https://www.corp.google.com/~mmenke/error.htm on
Chrome for Android (which doesn't have any media queries, so I assume it's
the desktop version of the ifdefs?), and it looked fine there. I've
attached a photo comparing how it looks on desktop to a Nexus 4. You'll
notice that the physical font sizes are slightly smaller on the phone, but
they're in proportion to the omnibox font size which is also smaller. And
smaller is in general correct - it's because as explained earlier phones
are held almost twice as close to the user's eyes as a desktop monitor
(unlike this photo, where they're the same distance from the camera lens),
hence the perceived size is the same.

I do however find the debug text ("Unable ... RESOLVED") a bit hard to read
on both desktop and phone (roughly equally hard to read on both), and would
support increasing the font size slightly on both.

> The mobile mocks also have a lot more spacing around the buttons, because
a
> button you touch generally needs to be larger than one you click on.

Yup, that's one thing that does need to be different on mobile.

> Again, using the larger buttons on desktop looks really weird when
compared to, say, about:settings.

I suspect over time as "desktop" starts including more touchscreen devices,
we'll eventually become accustomed to large button sizes there too. But I
agree that for now it'd be strange to have large buttons on non-touchscreen
desktop.


On Tue, Mar 12, 2013 at 1:49 PM, <mmenke@chromium.org> wrote:

> I'm adhering to mocks that I did not create.  The font sizes on
> Android/iOS had
> larger CSS "px" sizes than on desktop.  Mobile folks created the mobile
> mocks.
>
> Moreover, viewing the error page with the "same" "px" size on both mobile
> and
> desktop, the decision makes sense.  The error pages are near illegible
> with the
> "same" size on mobile as on desktop, and the text on desktop seems
> abnormally
> large when using the mobile font size.
>
> The mobile mocks also have a lot more spacing around the buttons, because a
> button you touch generally needs to be larger than one you click on.

[mmenke, 2013-03-12 14:44:10]

You're probably looking at the wrong version of the page.  That one
detected touch support and used larger fonts in that case.  Try reloading
it.


On Tue, Mar 12, 2013 at 10:42 AM, John Mellor <johnme@chromium.org> wrote:

> > I'm adhering to mocks that I did not create.  The font sizes on
> Android/iOS had larger CSS "px" sizes than on desktop.
>
> Were the designers definitely specifying sizes in CSS pixels not physical
> screen pixels?
>
> > The error pages are near illegible with the "same" size on mobile as on
> desktop,
> > and the text on desktop seems abnormally large when using the mobile
> font size.
>
> I could believe up to a ~30% difference, but "near illegible" seems very
> strange. I took at look at https://www.corp.google.com/~mmenke/error.htm on
> Chrome for Android (which doesn't have any media queries, so I assume it's
> the desktop version of the ifdefs?), and it looked fine there. I've
> attached a photo comparing how it looks on desktop to a Nexus 4. You'll
> notice that the physical font sizes are slightly smaller on the phone, but
> they're in proportion to the omnibox font size which is also smaller. And
> smaller is in general correct - it's because as explained earlier phones
> are held almost twice as close to the user's eyes as a desktop monitor
> (unlike this photo, where they're the same distance from the camera lens),
> hence the perceived size is the same.
>
> I do however find the debug text ("Unable ... RESOLVED") a bit hard to
> read on both desktop and phone (roughly equally hard to read on both), and
> would support increasing the font size slightly on both.
>
> > The mobile mocks also have a lot more spacing around the buttons,
> because a
> > button you touch generally needs to be larger than one you click on.
>
> Yup, that's one thing that does need to be different on mobile.
>
> > Again, using the larger buttons on desktop looks really weird when
> compared to, say, about:settings.
>
> I suspect over time as "desktop" starts including more touchscreen
> devices, we'll eventually become accustomed to large button sizes there
> too. But I agree that for now it'd be strange to have large buttons on
> non-touchscreen desktop.
>
>
> On Tue, Mar 12, 2013 at 1:49 PM, <mmenke@chromium.org> wrote:
>
>> I'm adhering to mocks that I did not create.  The font sizes on
>> Android/iOS had
>> larger CSS "px" sizes than on desktop.  Mobile folks created the mobile
>> mocks.
>>
>> Moreover, viewing the error page with the "same" "px" size on both mobile
>> and
>> desktop, the decision makes sense.  The error pages are near illegible
>> with the
>> "same" size on mobile as on desktop, and the text on desktop seems
>> abnormally
>> large when using the mobile font size.
>>
>> The mobile mocks also have a lot more spacing around the buttons, because
>> a
>> button you touch generally needs to be larger than one you click on.
>
>

[mmenke, 2013-03-12 14:44:58]

And "near illegible" might be exaggerating a little, but it's definitely
small enough to not be comfortable to read.


On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org> wrote:

> You're probably looking at the wrong version of the page.  That one
> detected touch support and used larger fonts in that case.  Try reloading
> it.
>
>
> On Tue, Mar 12, 2013 at 10:42 AM, John Mellor <johnme@chromium.org> wrote:
>
>> > I'm adhering to mocks that I did not create.  The font sizes on
>> Android/iOS had larger CSS "px" sizes than on desktop.
>>
>> Were the designers definitely specifying sizes in CSS pixels not physical
>> screen pixels?
>>
>> > The error pages are near illegible with the "same" size on mobile as on
>> desktop,
>> > and the text on desktop seems abnormally large when using the mobile
>> font size.
>>
>> I could believe up to a ~30% difference, but "near illegible" seems very
>> strange. I took at look at https://www.corp.google.com/~mmenke/error.htm on
>> Chrome for Android (which doesn't have any media queries, so I assume it's
>> the desktop version of the ifdefs?), and it looked fine there. I've
>> attached a photo comparing how it looks on desktop to a Nexus 4. You'll
>> notice that the physical font sizes are slightly smaller on the phone, but
>> they're in proportion to the omnibox font size which is also smaller. And
>> smaller is in general correct - it's because as explained earlier phones
>> are held almost twice as close to the user's eyes as a desktop monitor
>> (unlike this photo, where they're the same distance from the camera lens),
>> hence the perceived size is the same.
>>
>> I do however find the debug text ("Unable ... RESOLVED") a bit hard to
>> read on both desktop and phone (roughly equally hard to read on both), and
>> would support increasing the font size slightly on both.
>>
>> > The mobile mocks also have a lot more spacing around the buttons,
>> because a
>> > button you touch generally needs to be larger than one you click on.
>>
>> Yup, that's one thing that does need to be different on mobile.
>>
>> > Again, using the larger buttons on desktop looks really weird when
>> compared to, say, about:settings.
>>
>> I suspect over time as "desktop" starts including more touchscreen
>> devices, we'll eventually become accustomed to large button sizes there
>> too. But I agree that for now it'd be strange to have large buttons on
>> non-touchscreen desktop.
>>
>>
>> On Tue, Mar 12, 2013 at 1:49 PM, <mmenke@chromium.org> wrote:
>>
>>> I'm adhering to mocks that I did not create.  The font sizes on
>>> Android/iOS had
>>> larger CSS "px" sizes than on desktop.  Mobile folks created the mobile
>>> mocks.
>>>
>>> Moreover, viewing the error page with the "same" "px" size on both
>>> mobile and
>>> desktop, the decision makes sense.  The error pages are near illegible
>>> with the
>>> "same" size on mobile as on desktop, and the text on desktop seems
>>> abnormally
>>> large when using the mobile font size.
>>>
>>> The mobile mocks also have a lot more spacing around the buttons,
>>> because a
>>> button you touch generally needs to be larger than one you click on.
>>
>>
>

[mmenke, 2013-03-12 14:46:07]

And since you're using the pixel, which has touch, you're seeing the
"mobile" version on it, too, which uses strangely large fonts (Just compare
to about:settings).


On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org> wrote:

> And "near illegible" might be exaggerating a little, but it's definitely
> small enough to not be comfortable to read.
>
>
> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org> wrote:
>
>> You're probably looking at the wrong version of the page.  That one
>> detected touch support and used larger fonts in that case.  Try reloading
>> it.
>>
>>
>> On Tue, Mar 12, 2013 at 10:42 AM, John Mellor <johnme@chromium.org>wrote:
>>
>>> > I'm adhering to mocks that I did not create.  The font sizes on
>>> Android/iOS had larger CSS "px" sizes than on desktop.
>>>
>>> Were the designers definitely specifying sizes in CSS pixels not
>>> physical screen pixels?
>>>
>>> > The error pages are near illegible with the "same" size on mobile as
>>> on desktop,
>>> > and the text on desktop seems abnormally large when using the mobile
>>> font size.
>>>
>>> I could believe up to a ~30% difference, but "near illegible" seems very
>>> strange. I took at look at https://www.corp.google.com/~mmenke/error.htm on
>>> Chrome for Android (which doesn't have any media queries, so I assume it's
>>> the desktop version of the ifdefs?), and it looked fine there. I've
>>> attached a photo comparing how it looks on desktop to a Nexus 4. You'll
>>> notice that the physical font sizes are slightly smaller on the phone, but
>>> they're in proportion to the omnibox font size which is also smaller. And
>>> smaller is in general correct - it's because as explained earlier phones
>>> are held almost twice as close to the user's eyes as a desktop monitor
>>> (unlike this photo, where they're the same distance from the camera lens),
>>> hence the perceived size is the same.
>>>
>>> I do however find the debug text ("Unable ... RESOLVED") a bit hard to
>>> read on both desktop and phone (roughly equally hard to read on both), and
>>> would support increasing the font size slightly on both.
>>>
>>> > The mobile mocks also have a lot more spacing around the buttons,
>>> because a
>>> > button you touch generally needs to be larger than one you click on.
>>>
>>> Yup, that's one thing that does need to be different on mobile.
>>>
>>> > Again, using the larger buttons on desktop looks really weird when
>>> compared to, say, about:settings.
>>>
>>> I suspect over time as "desktop" starts including more touchscreen
>>> devices, we'll eventually become accustomed to large button sizes there
>>> too. But I agree that for now it'd be strange to have large buttons on
>>> non-touchscreen desktop.
>>>
>>>
>>> On Tue, Mar 12, 2013 at 1:49 PM, <mmenke@chromium.org> wrote:
>>>
>>>> I'm adhering to mocks that I did not create.  The font sizes on
>>>> Android/iOS had
>>>> larger CSS "px" sizes than on desktop.  Mobile folks created the mobile
>>>> mocks.
>>>>
>>>> Moreover, viewing the error page with the "same" "px" size on both
>>>> mobile and
>>>> desktop, the decision makes sense.  The error pages are near illegible
>>>> with the
>>>> "same" size on mobile as on desktop, and the text on desktop seems
>>>> abnormally
>>>> large when using the mobile font size.
>>>>
>>>> The mobile mocks also have a lot more spacing around the buttons,
>>>> because a
>>>> button you touch generally needs to be larger than one you click on.
>>>
>>>
>>
>

[johnme, 2013-03-12 15:04:44]

While we're discussing this page, is this the best UI for DNS errors?
Telling the user "This webpage is not available" suggests that the page
doesn't exist or the server is having problems, when it's much more likely
that their connection is down (or is this only shown if the network stack
can successfully resolve other domain names?). Wouldn't something like the
following be more useful?:

*Your device is offline*
The page will be loaded when the network connection is restored.
[Reload now] [More]
<footer text goes here>

> You're probably looking at the wrong version of the page.

I saved the page to Drive and loaded that exact HTML, so since the page
doesn't have any media queries that change font size they should be
identical. I've tried with the new version of the page, and I find the text
there (specifically "The webpage at ... new web address." unpleasantly
small on both desktop and phone; again about equally so).

> And since you're using the pixel, which has touch, you're seeing the
"mobile" version

I didn't use a Pixel, I used Chrome Linux with a 30" desktop monitor (and a
Nexus 4).


On Tue, Mar 12, 2013 at 2:45 PM, Matt Menke <mmenke@chromium.org> wrote:

> And since you're using the pixel, which has touch, you're seeing the
> "mobile" version on it, too, which uses strangely large fonts (Just compare
> to about:settings).
>
>
> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org> wrote:
>
>> And "near illegible" might be exaggerating a little, but it's definitely
>> small enough to not be comfortable to read.
>>
>>
>> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org> wrote:
>>
>>> You're probably looking at the wrong version of the page.  That one
>>> detected touch support and used larger fonts in that case.  Try reloading
>>> it.
>>>
>>>
>>> On Tue, Mar 12, 2013 at 10:42 AM, John Mellor <johnme@chromium.org>wrote:
>>>
>>>> > I'm adhering to mocks that I did not create.  The font sizes on
>>>> Android/iOS had larger CSS "px" sizes than on desktop.
>>>>
>>>> Were the designers definitely specifying sizes in CSS pixels not
>>>> physical screen pixels?
>>>>
>>>> > The error pages are near illegible with the "same" size on mobile as
>>>> on desktop,
>>>> > and the text on desktop seems abnormally large when using the mobile
>>>> font size.
>>>>
>>>> I could believe up to a ~30% difference, but "near illegible" seems
>>>> very strange. I took at look at
>>>> https://www.corp.google.com/~mmenke/error.htm on Chrome for Android
>>>> (which doesn't have any media queries, so I assume it's the desktop
version
>>>> of the ifdefs?), and it looked fine there. I've attached a photo comparing
>>>> how it looks on desktop to a Nexus 4. You'll notice that the physical font
>>>> sizes are slightly smaller on the phone, but they're in proportion to the
>>>> omnibox font size which is also smaller. And smaller is in general correct
>>>> - it's because as explained earlier phones are held almost twice as close
>>>> to the user's eyes as a desktop monitor (unlike this photo, where they're
>>>> the same distance from the camera lens), hence the perceived size is the
>>>> same.
>>>>
>>>> I do however find the debug text ("Unable ... RESOLVED") a bit hard to
>>>> read on both desktop and phone (roughly equally hard to read on both), and
>>>> would support increasing the font size slightly on both.
>>>>
>>>> > The mobile mocks also have a lot more spacing around the buttons,
>>>> because a
>>>> > button you touch generally needs to be larger than one you click on.
>>>>
>>>> Yup, that's one thing that does need to be different on mobile.
>>>>
>>>> > Again, using the larger buttons on desktop looks really weird when
>>>> compared to, say, about:settings.
>>>>
>>>> I suspect over time as "desktop" starts including more touchscreen
>>>> devices, we'll eventually become accustomed to large button sizes there
>>>> too. But I agree that for now it'd be strange to have large buttons on
>>>> non-touchscreen desktop.
>>>>
>>>>
>>>> On Tue, Mar 12, 2013 at 1:49 PM, <mmenke@chromium.org> wrote:
>>>>
>>>>> I'm adhering to mocks that I did not create.  The font sizes on
>>>>> Android/iOS had
>>>>> larger CSS "px" sizes than on desktop.  Mobile folks created the
>>>>> mobile mocks.
>>>>>
>>>>> Moreover, viewing the error page with the "same" "px" size on both
>>>>> mobile and
>>>>> desktop, the decision makes sense.  The error pages are near illegible
>>>>> with the
>>>>> "same" size on mobile as on desktop, and the text on desktop seems
>>>>> abnormally
>>>>> large when using the mobile font size.
>>>>>
>>>>> The mobile mocks also have a lot more spacing around the buttons,
>>>>> because a
>>>>> button you touch generally needs to be larger than one you click on.
>>>>
>>>>
>>>
>>
>

[mmenke, 2013-03-12 15:14:05]

On Tue, Mar 12, 2013 at 11:04 AM, John Mellor <johnme@chromium.org> wrote:

> While we're discussing this page, is this the best UI for DNS errors?
> Telling the user "This webpage is not available" suggests that the page
> doesn't exist or the server is having problems, when it's much more likely
> that their connection is down (or is this only shown if the network stack
> can successfully resolve other domain names?). Wouldn't something like the
> following be more useful?:
>
> *Your device is offline*
> The page will be loaded when the network connection is restored.
> [Reload now] [More]
> <footer text goes here>
>

I don't know.  That's a ChromeOS page I've never seen before.


>  > You're probably looking at the wrong version of the page.
>
> I saved the page to Drive and loaded that exact HTML, so since the page
> doesn't have any media queries that change font size they should be
> identical. I've tried with the new version of the page, and I find the text
> there (specifically "The webpage at ... new web address." unpleasantly
> small on both desktop and phone; again about equally so).
>

The desktop page uses about the same font size as about:settings.  It may
actually be a little larger.


>  > And since you're using the pixel, which has touch, you're seeing the
> "mobile" version
>
> I didn't use a Pixel, I used Chrome Linux with a 30" desktop monitor (and
> a Nexus 4).
>

Sorry, may have been me experimenting with the mobile version on desktop.
 Regardless, that is the mobile version, and uses fonts larger than what's
used elsewhere in desktop Chrome.

The Android team landed the original change that used different font sizes
on mobile - https://chromiumcodereview.appspot.com/10834378 when
upstreaming.  I'm not changing anything here.


>
>
> On Tue, Mar 12, 2013 at 2:45 PM, Matt Menke <mmenke@chromium.org> wrote:
>
>> And since you're using the pixel, which has touch, you're seeing the
>> "mobile" version on it, too, which uses strangely large fonts (Just compare
>> to about:settings).
>>
>>
>> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org> wrote:
>>
>>> And "near illegible" might be exaggerating a little, but it's definitely
>>> small enough to not be comfortable to read.
>>>
>>>
>>> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org>wrote:
>>>
>>>> You're probably looking at the wrong version of the page.  That one
>>>> detected touch support and used larger fonts in that case.  Try reloading
>>>> it.
>>>>
>>>>
>>>> On Tue, Mar 12, 2013 at 10:42 AM, John Mellor <johnme@chromium.org>wrote:
>>>>
>>>>> > I'm adhering to mocks that I did not create.  The font sizes on
>>>>> Android/iOS had larger CSS "px" sizes than on desktop.
>>>>>
>>>>> Were the designers definitely specifying sizes in CSS pixels not
>>>>> physical screen pixels?
>>>>>
>>>>> > The error pages are near illegible with the "same" size on mobile as
>>>>> on desktop,
>>>>> > and the text on desktop seems abnormally large when using the mobile
>>>>> font size.
>>>>>
>>>>> I could believe up to a ~30% difference, but "near illegible" seems
>>>>> very strange. I took at look at
>>>>> https://www.corp.google.com/~mmenke/error.htm on Chrome for Android
>>>>> (which doesn't have any media queries, so I assume it's the desktop
version
>>>>> of the ifdefs?), and it looked fine there. I've attached a photo
comparing
>>>>> how it looks on desktop to a Nexus 4. You'll notice that the physical
font
>>>>> sizes are slightly smaller on the phone, but they're in proportion to the
>>>>> omnibox font size which is also smaller. And smaller is in general
correct
>>>>> - it's because as explained earlier phones are held almost twice as close
>>>>> to the user's eyes as a desktop monitor (unlike this photo, where they're
>>>>> the same distance from the camera lens), hence the perceived size is the
>>>>> same.
>>>>>
>>>>> I do however find the debug text ("Unable ... RESOLVED") a bit hard to
>>>>> read on both desktop and phone (roughly equally hard to read on both),
and
>>>>> would support increasing the font size slightly on both.
>>>>>
>>>>> > The mobile mocks also have a lot more spacing around the buttons,
>>>>> because a
>>>>> > button you touch generally needs to be larger than one you click on.
>>>>>
>>>>> Yup, that's one thing that does need to be different on mobile.
>>>>>
>>>>> > Again, using the larger buttons on desktop looks really weird when
>>>>> compared to, say, about:settings.
>>>>>
>>>>> I suspect over time as "desktop" starts including more touchscreen
>>>>> devices, we'll eventually become accustomed to large button sizes there
>>>>> too. But I agree that for now it'd be strange to have large buttons on
>>>>> non-touchscreen desktop.
>>>>>
>>>>>
>>>>> On Tue, Mar 12, 2013 at 1:49 PM, <mmenke@chromium.org> wrote:
>>>>>
>>>>>> I'm adhering to mocks that I did not create.  The font sizes on
>>>>>> Android/iOS had
>>>>>> larger CSS "px" sizes than on desktop.  Mobile folks created the
>>>>>> mobile mocks.
>>>>>>
>>>>>> Moreover, viewing the error page with the "same" "px" size on both
>>>>>> mobile and
>>>>>> desktop, the decision makes sense.  The error pages are near
>>>>>> illegible with the
>>>>>> "same" size on mobile as on desktop, and the text on desktop seems
>>>>>> abnormally
>>>>>> large when using the mobile font size.
>>>>>>
>>>>>> The mobile mocks also have a lot more spacing around the buttons,
>>>>>> because a
>>>>>> button you touch generally needs to be larger than one you click on.
>>>>>
>>>>>
>>>>
>>>
>>
>

[mmenke, 2013-03-12 15:41:42]

On 2013/03/12 15:14:05, mmenke wrote:
> On Tue, Mar 12, 2013 at 11:04 AM, John Mellor <mailto:johnme@chromium.org>
wrote:
> 
> > While we're discussing this page, is this the best UI for DNS errors?
> > Telling the user "This webpage is not available" suggests that the page
> > doesn't exist or the server is having problems, when it's much more likely
> > that their connection is down (or is this only shown if the network stack
> > can successfully resolve other domain names?). Wouldn't something like the
> > following be more useful?:
> >
> > *Your device is offline*
> > The page will be loaded when the network connection is restored.
> > [Reload now] [More]
> > <footer text goes here>
> >
> 
> I don't know.  That's a ChromeOS page I've never seen before.

Oh, sorry, was thinking you were referring to the ChromeOS reload page.

We actually detect when your device has no network connections and return a
different error page.  So it's unclear if you really are offline.  The
getaddrinfo error codes are completely useless.  There's an effort to try and
improve this with some probes, but that's a bit complicated.

> 
> 
> >  > You're probably looking at the wrong version of the page.
> >
> > I saved the page to Drive and loaded that exact HTML, so since the page
> > doesn't have any media queries that change font size they should be
> > identical. I've tried with the new version of the page, and I find the text
> > there (specifically "The webpage at ... new web address." unpleasantly
> > small on both desktop and phone; again about equally so).
> >
> 
> The desktop page uses about the same font size as about:settings.  It may
> actually be a little larger.
> 
> 
> >  > And since you're using the pixel, which has touch, you're seeing the
> > "mobile" version
> >
> > I didn't use a Pixel, I used Chrome Linux with a 30" desktop monitor (and
> > a Nexus 4).
> >
> 
> Sorry, may have been me experimenting with the mobile version on desktop.
>  Regardless, that is the mobile version, and uses fonts larger than what's
> used elsewhere in desktop Chrome.
> 
> The Android team landed the original change that used different font sizes
> on mobile - https://chromiumcodereview.appspot.com/10834378 when
> upstreaming.  I'm not changing anything here.
> 
> 
> >
> >
> > On Tue, Mar 12, 2013 at 2:45 PM, Matt Menke <mailto:mmenke@chromium.org>
wrote:
> >
> >> And since you're using the pixel, which has touch, you're seeing the
> >> "mobile" version on it, too, which uses strangely large fonts (Just compare
> >> to about:settings).
> >>
> >>
> >> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mailto:mmenke@chromium.org>
wrote:
> >>
> >>> And "near illegible" might be exaggerating a little, but it's definitely
> >>> small enough to not be comfortable to read.
> >>>
> >>>
> >>> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org>wrote:
> >>>
> >>>> You're probably looking at the wrong version of the page.  That one
> >>>> detected touch support and used larger fonts in that case.  Try reloading
> >>>> it.
> >>>>
> >>>>
> >>>> On Tue, Mar 12, 2013 at 10:42 AM, John Mellor <johnme@chromium.org>wrote:
> >>>>
> >>>>> > I'm adhering to mocks that I did not create.  The font sizes on
> >>>>> Android/iOS had larger CSS "px" sizes than on desktop.
> >>>>>
> >>>>> Were the designers definitely specifying sizes in CSS pixels not
> >>>>> physical screen pixels?
> >>>>>
> >>>>> > The error pages are near illegible with the "same" size on mobile as
> >>>>> on desktop,
> >>>>> > and the text on desktop seems abnormally large when using the mobile
> >>>>> font size.
> >>>>>
> >>>>> I could believe up to a ~30% difference, but "near illegible" seems
> >>>>> very strange. I took at look at
> >>>>> https://www.corp.google.com/%7Emmenke/error.htm on Chrome for Android
> >>>>> (which doesn't have any media queries, so I assume it's the desktop
> version
> >>>>> of the ifdefs?), and it looked fine there. I've attached a photo
> comparing
> >>>>> how it looks on desktop to a Nexus 4. You'll notice that the physical
> font
> >>>>> sizes are slightly smaller on the phone, but they're in proportion to
the
> >>>>> omnibox font size which is also smaller. And smaller is in general
> correct
> >>>>> - it's because as explained earlier phones are held almost twice as
close
> >>>>> to the user's eyes as a desktop monitor (unlike this photo, where
they're
> >>>>> the same distance from the camera lens), hence the perceived size is the
> >>>>> same.
> >>>>>
> >>>>> I do however find the debug text ("Unable ... RESOLVED") a bit hard to
> >>>>> read on both desktop and phone (roughly equally hard to read on both),
> and
> >>>>> would support increasing the font size slightly on both.
> >>>>>
> >>>>> > The mobile mocks also have a lot more spacing around the buttons,
> >>>>> because a
> >>>>> > button you touch generally needs to be larger than one you click on.
> >>>>>
> >>>>> Yup, that's one thing that does need to be different on mobile.
> >>>>>
> >>>>> > Again, using the larger buttons on desktop looks really weird when
> >>>>> compared to, say, about:settings.
> >>>>>
> >>>>> I suspect over time as "desktop" starts including more touchscreen
> >>>>> devices, we'll eventually become accustomed to large button sizes there
> >>>>> too. But I agree that for now it'd be strange to have large buttons on
> >>>>> non-touchscreen desktop.
> >>>>>
> >>>>>
> >>>>> On Tue, Mar 12, 2013 at 1:49 PM, <mailto:mmenke@chromium.org> wrote:
> >>>>>
> >>>>>> I'm adhering to mocks that I did not create.  The font sizes on
> >>>>>> Android/iOS had
> >>>>>> larger CSS "px" sizes than on desktop.  Mobile folks created the
> >>>>>> mobile mocks.
> >>>>>>
> >>>>>> Moreover, viewing the error page with the "same" "px" size on both
> >>>>>> mobile and
> >>>>>> desktop, the decision makes sense.  The error pages are near
> >>>>>> illegible with the
> >>>>>> "same" size on mobile as on desktop, and the text on desktop seems
> >>>>>> abnormally
> >>>>>> large when using the mobile font size.
> >>>>>>
> >>>>>> The mobile mocks also have a lot more spacing around the buttons,
> >>>>>> because a
> >>>>>> button you touch generally needs to be larger than one you click on.
> >>>>>
> >>>>>
> >>>>
> >>>
> >>
> >

[johnme, 2013-03-12 15:46:56]

> The desktop page uses about the same font size as about:settings.  It may
actually be a little larger.

Technically, on Ubuntu, about:settings uses 12px Ubuntu instead of 12px
Arial. Now 12px Ubuntu has an x-height of 7px, whereas 12px Arial has an
x-height of 6px, i.e. Ubuntu is 16.7% bigger. Obviously that depends on
font-hinting etc; at much larger sizes Ubuntu only seems about 5% bigger
than Arial.


On Tue, Mar 12, 2013 at 3:14 PM, Matt Menke <mmenke@chromium.org> wrote:

> On Tue, Mar 12, 2013 at 11:04 AM, John Mellor <johnme@chromium.org> wrote:
>
>> While we're discussing this page, is this the best UI for DNS errors?
>> Telling the user "This webpage is not available" suggests that the page
>> doesn't exist or the server is having problems, when it's much more likely
>> that their connection is down (or is this only shown if the network stack
>> can successfully resolve other domain names?). Wouldn't something like the
>> following be more useful?:
>>
>> *Your device is offline*
>> The page will be loaded when the network connection is restored.
>> [Reload now] [More]
>> <footer text goes here>
>>
>
> I don't know.  That's a ChromeOS page I've never seen before.
>
>
>>  > You're probably looking at the wrong version of the page.
>>
>> I saved the page to Drive and loaded that exact HTML, so since the page
>> doesn't have any media queries that change font size they should be
>> identical. I've tried with the new version of the page, and I find the text
>> there (specifically "The webpage at ... new web address." unpleasantly
>> small on both desktop and phone; again about equally so).
>>
>
> The desktop page uses about the same font size as about:settings.  It may
> actually be a little larger.
>
>
>>  > And since you're using the pixel, which has touch, you're seeing the
>> "mobile" version
>>
>> I didn't use a Pixel, I used Chrome Linux with a 30" desktop monitor (and
>> a Nexus 4).
>>
>
> Sorry, may have been me experimenting with the mobile version on desktop.
>  Regardless, that is the mobile version, and uses fonts larger than what's
> used elsewhere in desktop Chrome.
>
> The Android team landed the original change that used different font sizes
> on mobile - https://chromiumcodereview.appspot.com/10834378 when
> upstreaming.  I'm not changing anything here.
>
>
>>
>>
>> On Tue, Mar 12, 2013 at 2:45 PM, Matt Menke <mmenke@chromium.org> wrote:
>>
>>> And since you're using the pixel, which has touch, you're seeing the
>>> "mobile" version on it, too, which uses strangely large fonts (Just compare
>>> to about:settings).
>>>
>>>
>>> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org>wrote:
>>>
>>>> And "near illegible" might be exaggerating a little, but it's
>>>> definitely small enough to not be comfortable to read.
>>>>
>>>>
>>>> On Tue, Mar 12, 2013 at 10:44 AM, Matt Menke <mmenke@chromium.org>wrote:
>>>>
>>>>> You're probably looking at the wrong version of the page.  That one
>>>>> detected touch support and used larger fonts in that case.  Try reloading
>>>>> it.
>>>>>
>>>>>
>>>>> On Tue, Mar 12, 2013 at 10:42 AM, John Mellor <johnme@chromium.org>wrote:
>>>>>
>>>>>> > I'm adhering to mocks that I did not create.  The font sizes on
>>>>>> Android/iOS had larger CSS "px" sizes than on desktop.
>>>>>>
>>>>>> Were the designers definitely specifying sizes in CSS pixels not
>>>>>> physical screen pixels?
>>>>>>
>>>>>> > The error pages are near illegible with the "same" size on mobile
>>>>>> as on desktop,
>>>>>> > and the text on desktop seems abnormally large when using the
>>>>>> mobile font size.
>>>>>>
>>>>>> I could believe up to a ~30% difference, but "near illegible" seems
>>>>>> very strange. I took at look at
>>>>>> https://www.corp.google.com/~mmenke/error.htm on Chrome for Android
>>>>>> (which doesn't have any media queries, so I assume it's the desktop
version
>>>>>> of the ifdefs?), and it looked fine there. I've attached a photo
comparing
>>>>>> how it looks on desktop to a Nexus 4. You'll notice that the physical
font
>>>>>> sizes are slightly smaller on the phone, but they're in proportion to
the
>>>>>> omnibox font size which is also smaller. And smaller is in general
correct
>>>>>> - it's because as explained earlier phones are held almost twice as
close
>>>>>> to the user's eyes as a desktop monitor (unlike this photo, where
they're
>>>>>> the same distance from the camera lens), hence the perceived size is the
>>>>>> same.
>>>>>>
>>>>>> I do however find the debug text ("Unable ... RESOLVED") a bit hard
>>>>>> to read on both desktop and phone (roughly equally hard to read on
both),
>>>>>> and would support increasing the font size slightly on both.
>>>>>>
>>>>>> > The mobile mocks also have a lot more spacing around the buttons,
>>>>>> because a
>>>>>> > button you touch generally needs to be larger than one you click on.
>>>>>>
>>>>>> Yup, that's one thing that does need to be different on mobile.
>>>>>>
>>>>>> > Again, using the larger buttons on desktop looks really weird when
>>>>>> compared to, say, about:settings.
>>>>>>
>>>>>> I suspect over time as "desktop" starts including more touchscreen
>>>>>> devices, we'll eventually become accustomed to large button sizes there
>>>>>> too. But I agree that for now it'd be strange to have large buttons on
>>>>>> non-touchscreen desktop.
>>>>>>
>>>>>>
>>>>>> On Tue, Mar 12, 2013 at 1:49 PM, <mmenke@chromium.org> wrote:
>>>>>>
>>>>>>> I'm adhering to mocks that I did not create.  The font sizes on
>>>>>>> Android/iOS had
>>>>>>> larger CSS "px" sizes than on desktop.  Mobile folks created the
>>>>>>> mobile mocks.
>>>>>>>
>>>>>>> Moreover, viewing the error page with the "same" "px" size on both
>>>>>>> mobile and
>>>>>>> desktop, the decision makes sense.  The error pages are near
>>>>>>> illegible with the
>>>>>>> "same" size on mobile as on desktop, and the text on desktop seems
>>>>>>> abnormally
>>>>>>> large when using the mobile font size.
>>>>>>>
>>>>>>> The mobile mocks also have a lot more spacing around the buttons,
>>>>>>> because a
>>>>>>> button you touch generally needs to be larger than one you click on.
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

[mmenke, 2013-03-12 15:56:28]

On Tue, Mar 12, 2013 at 11:46 AM, John Mellor <johnme@chromium.org> wrote:

> > The desktop page uses about the same font size as about:settings.  It
> may actually be a little larger.
>
> Technically, on Ubuntu, about:settings uses 12px Ubuntu instead of 12px
> Arial. Now 12px Ubuntu has an x-height of 7px, whereas 12px Arial has an
> x-height of 6px, i.e. Ubuntu is 16.7% bigger. Obviously that depends on
> font-hinting etc; at much larger sizes Ubuntu only seems about 5% bigger
> than Arial.
>

On Windows, our most popular platform, they both are exactly the same
height (9 pixels from the bottom of the average capital letter to the top).


On Tue, Mar 12, 2013 at 11:49 AM, John Mellor <johnme@chromium.org> wrote:

> -cc: others on this detail
>
> > The Android team landed the original change that used different font
> sizes on mobile - https://chromiumcodereview.appspot.com/10834378 when
> upstreaming.
>
> Technically that patch doesn't change any font sizes, though I'm happy to
> believe that one of their other patches did.
>

You're right...  I thought there were old <if>'s for differing font sizes,
but they were only for padding.

[mmenke, 2013-03-12 16:38:54]

On 2013/03/12 15:56:28, mmenke wrote:
> On Tue, Mar 12, 2013 at 11:46 AM, John Mellor <mailto:johnme@chromium.org>
wrote:
> 
> > > The desktop page uses about the same font size as about:settings.  It
> > may actually be a little larger.
> >
> > Technically, on Ubuntu, about:settings uses 12px Ubuntu instead of 12px
> > Arial. Now 12px Ubuntu has an x-height of 7px, whereas 12px Arial has an
> > x-height of 6px, i.e. Ubuntu is 16.7% bigger. Obviously that depends on
> > font-hinting etc; at much larger sizes Ubuntu only seems about 5% bigger
> > than Arial.
> >
> 
> On Windows, our most popular platform, they both are exactly the same
> height (9 pixels from the bottom of the average capital letter to the top).

I don't believe that the only disagreement here is due to the fact that the 12px
version of Arial on Ubuntu is strangely small, as Android doesn't seem to have
this issue.  However, I'm happy to increase the smaller desktop fonts to 13px. 
Unfortunately, I can't easily use the same font choice algorithm used by WebUI,
since it depends on GTK, and we need to pick the font in the Renderer process.

[mmenke, 2013-03-12 16:44:46]

On 2013/03/12 16:38:54, mmenke wrote:
> On 2013/03/12 15:56:28, mmenke wrote:
> > On Tue, Mar 12, 2013 at 11:46 AM, John Mellor <mailto:johnme@chromium.org>
> wrote:
> > 
> > > > The desktop page uses about the same font size as about:settings.  It
> > > may actually be a little larger.
> > >
> > > Technically, on Ubuntu, about:settings uses 12px Ubuntu instead of 12px
> > > Arial. Now 12px Ubuntu has an x-height of 7px, whereas 12px Arial has an
> > > x-height of 6px, i.e. Ubuntu is 16.7% bigger. Obviously that depends on
> > > font-hinting etc; at much larger sizes Ubuntu only seems about 5% bigger
> > > than Arial.
> > >
> > 
> > On Windows, our most popular platform, they both are exactly the same
> > height (9 pixels from the bottom of the average capital letter to the top).
> 
> I don't believe that the only disagreement here is due to the fact that the
12px
> version of Arial on Ubuntu is strangely small, as Android doesn't seem to have
> this issue.  However, I'm happy to increase the smaller desktop fonts to 13px.

> Unfortunately, I can't easily use the same font choice algorithm used by
WebUI,
> since it depends on GTK, and we need to pick the font in the Renderer process.

Ahh, nevermind.  Looks like it's doable.  I'll try it and see what comes out on
Android, but I suspect we'll run into the same issue with font sizes, just
Ubuntu will be fixed.

[mmenke, 2013-03-26 16:29:55]

I've gone ahead and switched to using the default platform font everywhere
(Which the page should have been doing in the first place), and the default font
size ("75%") for everything but the header.  I still think that this is too
small (It's effectively the same font and size as I was railing against), but
since I'm using the platform default, no longer consider this my problem.

Sorry for my slow response to feedback - I wanted to test what the page would
actually looked like on Android, and put off figuring out how to get the default
font / text size on Android for a while.

There's a sync between 14 and 15 that I should have done in a separate upload. 
It brought in a couple minor changes, sorry about that.

[mmenke, 2013-03-26 16:32:09]

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:50: font-weight: normal;
Without a font size, this is currently really ugly, but it's going away in a
followup CL.

[Nico, 2013-03-26 17:56:30]

Thanks, this looks much better. A bunch of nits, the only real comment is about
the js.

https://codereview.chromium.org/12277011/diff/106001/chrome/common/localized_...
File chrome/common/localized_error.cc (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/common/localized_...
chrome/common/localized_error.cc:35: #define IDS_ERRORPAGES_DETAILS_NULL 0
const int?

Also, since this is not a real string resource, I wouldn't name it like one.
(kErrorPagesNoDetails)

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:92: text-shadow: 0 1px 0
rgba(255,255,255,0.3);
No mobile perf concerns here?

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:100: background-image:
-webkit-linear-gradient(#f6f6f6 5%, #efefef 50%, #ddd);
Or here?

Unrelated: I think webkit supports the standardized, cross-browser unprefixed
gradients these days. We should probably use those?

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:103: border-radius: 2px;
border-radius (and box-shadow below) are also slow

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:177: function updateHelpBox() {
I think the more common way to toggle an animation is to have css classes for
the start and end states, and use classList.toggle() to switch between them (see
e.g.
http://tiffanybbrown.com/2011/09/26/toggle-blind-effect-with-css3-transitions/)

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:202: * Makes the reload and more / less
buttons have the same width.
This feels like something that might be doable more declaratively these days. I
tried getting something working and failed though.

+Tony: Do you know how to do a button layout like on
https://www.corp.google.com/~kenmoore/mocks/chrome/ErrorPages/Pass4/chrome-er...
(both buttons the same width) using css?

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:230: window.onresize = updateHelpBox;
Can you get rid of this if you use height: 100% in the expanded case?

[Nico, 2013-03-26 18:14:33]

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:202: * Makes the reload and more / less
buttons have the same width.
On 2013/03/26 17:56:31, Nico wrote:
> This feels like something that might be doable more declaratively these days.
I
> tried getting something working and failed though.
> 
> +Tony: Do you know how to do a button layout like on
>
https://www.corp.google.com/%7Ekenmoore/mocks/chrome/ErrorPages/Pass4/chrome-...
> (both buttons the same width) using css?

Actually, one can just Inspect Element on
https://www.corp.google.com/~kenmoore/mocks/chrome/ErrorPages/Pass4/chrome-er...
. Looks like min-width is deemed to be acceptable (which gives the buttons the
same width for short strings but not for longer ones)

[tony, 2013-03-26 18:33:04]

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:202: * Makes the reload and more / less
buttons have the same width.
You mean the Reload and More buttons on the 502 error page, right?

You can use flexbox (display: -webkit-flex) for this. or you could just do
something like width: 50% on both buttons.

Here's a flexbox example:
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...

[mmenke1, 2013-03-26 18:40:38]

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:202: * Makes the reload and more / less
buttons have the same width.
On 2013/03/26 18:33:05, tony wrote:
> You mean the Reload and More buttons on the 502 error page, right?
> 
> You can use flexbox (display: -webkit-flex) for this. or you could just do
> something like width: 50% on both buttons.
> 
> Here's a flexbox example:
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...

Since we're toggling the text in one of the buttons from one internationalized
string to another, I don't think flex boxes will prevent buttons from jittering
when toggling text in all cases, for all languages, unless we have a single
known maximum width for all localizations of the strings.

width: 50% seems like it would result in huge buttons (Using a smaller value
runs into the same need a known max-width problem).

Am I missing something?

[mmenke1, 2013-03-26 18:44:52]

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:202: * Makes the reload and more / less
buttons have the same width.
On 2013/03/26 18:40:39, mmenke (incorrect) wrote:
> On 2013/03/26 18:33:05, tony wrote:
> > You mean the Reload and More buttons on the 502 error page, right?
> > 
> > You can use flexbox (display: -webkit-flex) for this. or you could just do
> > something like width: 50% on both buttons.
> > 
> > Here's a flexbox example:
> >
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...
> 
> Since we're toggling the text in one of the buttons from one internationalized
> string to another, I don't think flex boxes will prevent buttons from
jittering
> when toggling text in all cases, for all languages, unless we have a single
> known maximum width for all localizations of the strings.
> 
> width: 50% seems like it would result in huge buttons (Using a smaller value
> runs into the same need a known max-width problem).
> 
> Am I missing something?

Oh, I am...  Flexbox to do the layout, then position separately will prevent
jitter of both buttons, since they're laid out separately.  I'll go ahead and do
that.

[Nico, 2013-03-26 18:45:26]

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:202: * Makes the reload and more / less
buttons have the same width.
On 2013/03/26 18:40:39, mmenke (incorrect) wrote:
> On 2013/03/26 18:33:05, tony wrote:
> > You mean the Reload and More buttons on the 502 error page, right?
> > 
> > You can use flexbox (display: -webkit-flex) for this. or you could just do
> > something like width: 50% on both buttons.
> > 
> > Here's a flexbox example:
> >
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...
> 
> Since we're toggling the text in one of the buttons from one internationalized
> string to another, I don't think flex boxes will prevent buttons from
jittering
> when toggling text in all cases, for all languages, unless we have a single
> known maximum width for all localizations of the strings.
> 
> width: 50% seems like it would result in huge buttons (Using a smaller value
> runs into the same need a known max-width problem).
> 
> Am I missing something?

I agree that max-width + flexbox doesn't work well. The mock uses min-width to
make sure the buttons have the same width for small strings and then gives the
buttons not the same width for longer strings, which I think probably looks
better than two buttons at the same width if the two button texts differ in
length.

[newt (away), 2013-03-26 18:47:54]

On 2013/03/26 18:45:26, Nico wrote:
>
https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
> File chrome/renderer/resources/neterror.html (right):
> 
>
https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
> chrome/renderer/resources/neterror.html:202: * Makes the reload and more /
less
> buttons have the same width.
> On 2013/03/26 18:40:39, mmenke (incorrect) wrote:
> > On 2013/03/26 18:33:05, tony wrote:
> > > You mean the Reload and More buttons on the 502 error page, right?
> > > 
> > > You can use flexbox (display: -webkit-flex) for this. or you could just do
> > > something like width: 50% on both buttons.
> > > 
> > > Here's a flexbox example:
> > >
> >
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...
> > 
> > Since we're toggling the text in one of the buttons from one
internationalized
> > string to another, I don't think flex boxes will prevent buttons from
> jittering
> > when toggling text in all cases, for all languages, unless we have a single
> > known maximum width for all localizations of the strings.
> > 
> > width: 50% seems like it would result in huge buttons (Using a smaller value
> > runs into the same need a known max-width problem).
> > 
> > Am I missing something?
> 
> I agree that max-width + flexbox doesn't work well. The mock uses min-width to
> make sure the buttons have the same width for small strings and then gives the
> buttons not the same width for longer strings, which I think probably looks
> better than two buttons at the same width if the two button texts differ in
> length.

I think the main constraint is making sure the "more/less" button doesn't change
size when the text changes.

[mmenke1, 2013-03-26 18:48:59]

On 2013/03/26 18:47:54, newt wrote:
> On 2013/03/26 18:45:26, Nico wrote:
> >
>
https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
> > File chrome/renderer/resources/neterror.html (right):
> > 
> >
>
https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
> > chrome/renderer/resources/neterror.html:202: * Makes the reload and more /
> less
> > buttons have the same width.
> > On 2013/03/26 18:40:39, mmenke (incorrect) wrote:
> > > On 2013/03/26 18:33:05, tony wrote:
> > > > You mean the Reload and More buttons on the 502 error page, right?
> > > > 
> > > > You can use flexbox (display: -webkit-flex) for this. or you could just
do
> > > > something like width: 50% on both buttons.
> > > > 
> > > > Here's a flexbox example:
> > > >
> > >
> >
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...
> > > 
> > > Since we're toggling the text in one of the buttons from one
> internationalized
> > > string to another, I don't think flex boxes will prevent buttons from
> > jittering
> > > when toggling text in all cases, for all languages, unless we have a
single
> > > known maximum width for all localizations of the strings.
> > > 
> > > width: 50% seems like it would result in huge buttons (Using a smaller
value
> > > runs into the same need a known max-width problem).
> > > 
> > > Am I missing something?
> > 
> > I agree that max-width + flexbox doesn't work well. The mock uses min-width
to
> > make sure the buttons have the same width for small strings and then gives
the
> > buttons not the same width for longer strings, which I think probably looks
> > better than two buttons at the same width if the two button texts differ in
> > length.
> 
> I think the main constraint is making sure the "more/less" button doesn't
change
> size when the text changes.

Which I don't believe is accomplished by any of the suggestions.

[mmenke, 2013-03-26 18:55:34]

On 2013/03/26 18:48:59, mmenke (incorrect) wrote:
> On 2013/03/26 18:47:54, newt wrote:
> > On 2013/03/26 18:45:26, Nico wrote:
> > >
> >
>
https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
> > > File chrome/renderer/resources/neterror.html (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
> > > chrome/renderer/resources/neterror.html:202: * Makes the reload and more /
> > less
> > > buttons have the same width.
> > > On 2013/03/26 18:40:39, mmenke (incorrect) wrote:
> > > > On 2013/03/26 18:33:05, tony wrote:
> > > > > You mean the Reload and More buttons on the 502 error page, right?
> > > > > 
> > > > > You can use flexbox (display: -webkit-flex) for this. or you could
just
> do
> > > > > something like width: 50% on both buttons.
> > > > > 
> > > > > Here's a flexbox example:
> > > > >
> > > >
> > >
> >
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...
> > > > 
> > > > Since we're toggling the text in one of the buttons from one
> > internationalized
> > > > string to another, I don't think flex boxes will prevent buttons from
> > > jittering
> > > > when toggling text in all cases, for all languages, unless we have a
> single
> > > > known maximum width for all localizations of the strings.
> > > > 
> > > > width: 50% seems like it would result in huge buttons (Using a smaller
> value
> > > > runs into the same need a known max-width problem).
> > > > 
> > > > Am I missing something?
> > > 
> > > I agree that max-width + flexbox doesn't work well. The mock uses
min-width
> to
> > > make sure the buttons have the same width for small strings and then gives
> the
> > > buttons not the same width for longer strings, which I think probably
looks
> > > better than two buttons at the same width if the two button texts differ
in
> > > length.
> > 
> > I think the main constraint is making sure the "more/less" button doesn't
> change
> > size when the text changes.
> 
> Which I don't believe is accomplished by any of the suggestions.

My suggestion:  I upload a version of this without the transition effects, and
with buttons that are will be resized and moved when their text changes (what
happens when I remove all my logic to do that stuff).  We get that committed,
and then can worry about those two features in another CL.  Or just leave them
in that state.

[mmenke, 2013-03-26 18:59:14]

On 2013/03/26 18:55:34, mmenke wrote:
> On 2013/03/26 18:48:59, mmenke (incorrect) wrote:
> > On 2013/03/26 18:47:54, newt wrote:
> > > On 2013/03/26 18:45:26, Nico wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
> > > > File chrome/renderer/resources/neterror.html (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
> > > > chrome/renderer/resources/neterror.html:202: * Makes the reload and more
/
> > > less
> > > > buttons have the same width.
> > > > On 2013/03/26 18:40:39, mmenke (incorrect) wrote:
> > > > > On 2013/03/26 18:33:05, tony wrote:
> > > > > > You mean the Reload and More buttons on the 502 error page, right?
> > > > > > 
> > > > > > You can use flexbox (display: -webkit-flex) for this. or you could
> just
> > do
> > > > > > something like width: 50% on both buttons.
> > > > > > 
> > > > > > Here's a flexbox example:
> > > > > >
> > > > >
> > > >
> > >
> >
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...
> > > > > 
> > > > > Since we're toggling the text in one of the buttons from one
> > > internationalized
> > > > > string to another, I don't think flex boxes will prevent buttons from
> > > > jittering
> > > > > when toggling text in all cases, for all languages, unless we have a
> > single
> > > > > known maximum width for all localizations of the strings.
> > > > > 
> > > > > width: 50% seems like it would result in huge buttons (Using a smaller
> > value
> > > > > runs into the same need a known max-width problem).
> > > > > 
> > > > > Am I missing something?
> > > > 
> > > > I agree that max-width + flexbox doesn't work well. The mock uses
> min-width
> > to
> > > > make sure the buttons have the same width for small strings and then
gives
> > the
> > > > buttons not the same width for longer strings, which I think probably
> looks
> > > > better than two buttons at the same width if the two button texts differ
> in
> > > > length.
> > > 
> > > I think the main constraint is making sure the "more/less" button doesn't
> > change
> > > size when the text changes.
> > 
> > Which I don't believe is accomplished by any of the suggestions.
> 
> My suggestion:  I upload a version of this without the transition effects, and
> with buttons that are will be resized and moved when their text changes (what
> happens when I remove all my logic to do that stuff).  We get that committed,
> and then can worry about those two features in another CL.  Or just leave them
> in that state.

Getting rid of the transition, too, because height: 100% doesn't work with the
transition, have to set it to a px value.

[mmenke, 2013-03-26 19:22:36]

https://codereview.chromium.org/12277011/diff/106001/chrome/common/localized_...
File chrome/common/localized_error.cc (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/common/localized_...
chrome/common/localized_error.cc:35: #define IDS_ERRORPAGES_DETAILS_NULL 0
On 2013/03/26 17:56:31, Nico wrote:
> const int?
> 
> Also, since this is not a real string resource, I wouldn't name it like one.
> (kErrorPagesNoDetails)

Done.

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
File chrome/renderer/resources/neterror.html (right):

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:92: text-shadow: 0 1px 0
rgba(255,255,255,0.3);
On 2013/03/26 17:56:31, Nico wrote:
> No mobile perf concerns here?

Good suggestions, done.

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:100: background-image:
-webkit-linear-gradient(#f6f6f6 5%, #efefef 50%, #ddd);
On 2013/03/26 17:56:31, Nico wrote:
> Or here?

I can't notice any significant perf difference (It only really matters here when
pressing the more/less button), and I think the difference in appearance is
worth a minimal loss of performance here.

> Unrelated: I think webkit supports the standardized, cross-browser unprefixed
> gradients these days. We should probably use those?

Done.  We do on Canary, not yet on stable.  Looking at iOS, which is sometimes
behind (And where we also use the error pages), looks like iOS also has support.

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:103: border-radius: 2px;
On 2013/03/26 17:56:31, Nico wrote:
> border-radius (and box-shadow below) are also slow

Done.

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:177: function updateHelpBox() {
On 2013/03/26 17:56:31, Nico wrote:
> I think the more common way to toggle an animation is to have css classes for
> the start and end states, and use classList.toggle() to switch between them
(see
> e.g.
>
http://tiffanybbrown.com/2011/09/26/toggle-blind-effect-with-css3-transitions/)

Done, though the class now just sets display:none.  Still have to update button
text, so keeping this function.

Could set display directly, but having it start hidden before a function is
called prevents, or at least reduces, blinking on load.

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:202: * Makes the reload and more / less
buttons have the same width.
On 2013/03/26 18:33:05, tony wrote:
> You mean the Reload and More buttons on the 502 error page, right?
> 
> You can use flexbox (display: -webkit-flex) for this. or you could just do
> something like width: 50% on both buttons.
> 
> Here's a flexbox example:
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...

Since we're toggling the text in one of the buttons from one internationalized
string to another, I don't think either of these will prevent buttons from
jittering when toggling text in all cases, for all languages, unless we have a
single known maximum width for all localizations of the strings.

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:202: * Makes the reload and more / less
buttons have the same width.
On 2013/03/26 18:45:26, Nico wrote:
> On 2013/03/26 18:40:39, mmenke (incorrect) wrote:
> > On 2013/03/26 18:33:05, tony wrote:
> > > You mean the Reload and More buttons on the 502 error page, right?
> > > 
> > > You can use flexbox (display: -webkit-flex) for this. or you could just do
> > > something like width: 50% on both buttons.
> > > 
> > > Here's a flexbox example:
> > >
> >
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwEOWEZDTEpVDkdNRlkcnqCi...
> > 
> > Since we're toggling the text in one of the buttons from one
internationalized
> > string to another, I don't think flex boxes will prevent buttons from
> jittering
> > when toggling text in all cases, for all languages, unless we have a single
> > known maximum width for all localizations of the strings.
> > 
> > width: 50% seems like it would result in huge buttons (Using a smaller value
> > runs into the same need a known max-width problem).
> > 
> > Am I missing something?
> 
> I agree that max-width + flexbox doesn't work well. The mock uses min-width to
> make sure the buttons have the same width for small strings and then gives the
> buttons not the same width for longer strings, which I think probably looks
> better than two buttons at the same width if the two button texts differ in
> length.

Code removed, not using a flexbox replacement, or any special sizing code.  Can
worry about it in a followup CL.

https://codereview.chromium.org/12277011/diff/106001/chrome/renderer/resource...
chrome/renderer/resources/neterror.html:230: window.onresize = updateHelpBox;
On 2013/03/26 17:56:31, Nico wrote:
> Can you get rid of this if you use height: 100% in the expanded case?

While that does make it just as tall as we want, it does not work with the
transition, unfortunately.  Transition removed.

[Nico, 2013-03-26 20:17:37]

lgtm

ps: The bug suggests that all error pages should be restyled. To prevent
shipping an inconsistent set of error pages, maybe it makes sense to put the new
versions behind a flag, so that they can be toggled in one go once they're done?

[mmenke1, 2013-03-26 20:34:07]

On 2013/03/26 20:17:37, Nico wrote:
> lgtm
> 
> ps: The bug suggests that all error pages should be restyled. To prevent
> shipping an inconsistent set of error pages, maybe it makes sense to put the
new
> versions behind a flag, so that they can be toggled in one go once they're
done?

Thanks for the review!

That's a reasonable suggestion, but I don't think it's worth the effort.

The only other error pages that actually share the old style with these are the
SSL interstitial warning pages.  The interstitials don't currently have any new
mocks, and it's not clear how similar we want the templates to be - those are
security-related error pages, and we want them to very clearly be different and
scary.

Also, one of the next CLs will involve a lot of string updates, and keeping the
old strings seems like a lot of overhead.  Since we're going from one string per
suggestion to two, can't easily just update the old strings.

I'll plan to land this tomorrow.

[commit-bot: I haz the power, 2013-03-27 14:27:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mmenke@chromium.org/12277011/105009

[mmenke, 2013-03-27 16:31:50]

On 2013/03/27 14:27:52, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/mmenke%40chromium.org/12277011/105009

Looks to me like we're running up against the sandbox when trying to get the
default font on Linux.  I had assumed since it was in common/, this would work. 
We may have to either go back to hard coding the fonts, or update the underlying
function on Linux to fall back to Arial.

I'll talk to the maintainers of that code to try and figure out the best thing
to do here.

[mmenke, 2013-04-01 20:56:09]

thakis:  I've had to go back to using hard-coded font face and size (I went back
to 10pt, the same size before this CL), since the function I was using to get it
calls into GTK on Linux, which doesn't work in the renderer process.

I want to switch to using the WebUI values, but after some digging, it seems
like I'll have to add a new IPC to get them in the renderer process, which will
need to be done in another CL.

The changes I've made to this CL are quite simple, basically reverting a couple
line to what they looked like before this CL, but I thought I'd give you chance
to take a look before I land them.

On 2013/03/27 16:31:50, mmenke wrote:
> On 2013/03/27 14:27:52, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> >
https://chromium-status.appspot.com/cq/mmenke%2540chromium.org/12277011/105009
> 
> Looks to me like we're running up against the sandbox when trying to get the
> default font on Linux.  I had assumed since it was in common/, this would
work. 
> We may have to either go back to hard coding the fonts, or update the
underlying
> function on Linux to fall back to Arial.
> 
> I'll talk to the maintainers of that code to try and figure out the best thing
> to do here.

[Nico, 2013-04-01 21:26:18]

still lgtm

[mmenke, 2013-04-01 21:33:09]

On 2013/04/01 21:26:18, Nico wrote:
> still lgtm

Thanks again (And sorry for the extra round of review)

[commit-bot: I haz the power, 2013-04-01 21:34:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mmenke@chromium.org/12277011/131001

[commit-bot: I haz the power, 2013-04-02 00:13:43]

Change committed as 191712