Bug 53707 – Viewport Warning/Error Messages Are Now Inaccurate

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Joseph Pecoraro]

The changes to the ViewportArguments implementation did not update the error messages
accordingly, so they either no longer make sense or are inaccurate.

The new implementation landed in r67376:
http://trac.webkit.org/changeset/67376

It follows the "css-viewport" spec:
http://people.opera.com/rune/TR/css-viewport/#parsing-algorithm

Example situations:

1. <meta name="viewport" content="width=nothing">

  Actual Result   => Viewport argument "width" not recognized. Content ignored.
  Expected Result => Viewport argument value for "width" not recognized. Value ignored.

2. <meta name="viewport" content="width=1.0x">

  Actual Result   => Viewport argument "width" not recognized. Content ignored.
  Expected Result => Viewport argument value for "width" was truncated from "1.0x" to "1.0".

3. <meta name="viewport" content="width=101">
   NOTE: With a device width that is NOT 101.
   NOTE: The same applies for height.

  Actual Result   => Viewport width or height set to physical device width, try using "device-width" constant instead for future compatibility.
  Expected Result => No warning, or a suggestion to use appropriate constants.
                  => The "device-width" / "device-height" message should only happen when the constant is equal to the device width/height.

4. Inconsistent spacing in Error messages:

  Two spaces after period => "Viewport argument \"%replacement\" not recognized. Content ignored."
  One space  after period => "Viewport maximum-scale cannot be larger than 10.0.  The maximum-scale will be set to 10.0."

[Joseph Pecoraro]

Created attachment 82739 [details]
[PATCH] Patch and Tests for Error messages

I don't have the ability to run tests on any of the ports that actually run
fast/viewport tests. Also, there might be something I specifically need
to do in order to enable CONSOLE output in these tests. But the tests
are as follows:

  1: <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=2">
      no warnings. Works as expected.

  2: <meta name="viewport" content="wwidth=100">
      ERROR: Viewport argument key "wwidth" not recognized and ignored.

  3: <meta name="viewport" content="width=unrecognized-width">
      ERROR: Viewport argument value "unrecognized-width" for key "width" not recognized. Content ignored.

  4. <meta name="viewport" content="width=123x456">
      TIP: Viewport argument value "123x456" for key "width" was truncated to its numeric prefix.

  5. <meta name="viewport" content="width=320, height=352">
      TIP: Viewport width or height set to physical device width, try using "device-width" constant instead for future compatibility.
      TIP: Viewport height or height set to physical device height, try using "device-height" constant instead for future compatibility.

  6. <meta name="viewport" content="width=device-width; initial-scale=1.0; maximum-scale=1.0; user-scalable=0;">
      ERROR: Viewport argument value "device-width;" for key "width" not recognized. Content ignored.
      TIP: Viewport argument value "1.0;" for key "initial-scale" was truncated to its numeric prefix.
      TIP: Viewport argument value "1.0;" for key "maximum-scale" was truncated to its numeric prefix.
      TIP: Viewport argument value "0;" for key "user-scalable" was truncated to its numeric prefix.

[Kenneth Rohde Christiansen]

Comment on attachment 82739 [details]
[PATCH] Patch and Tests for Error messages

View in context: https://bugs.webkit.org/attachment.cgi?id=82739&action=review

> LayoutTests/ChangeLog:8
> +        Added tests specifically to test Console warnings.

Great stuff!

[Joseph Pecoraro]

To land, this requires changes from a patch on:
<http://webkit.org/b/53705> Viewport parsing no longer accepts "1.0;" value as valid.

Also, I want to verify that I don't have to do anything special for these
tests to output console messages. It would be useless if these tests
didn't print the console warnings in the dump render tree results!

[Joseph Pecoraro]

Created attachment 83231 [details]
[PATCH] One more test for a missing value.

Kenneth, is it okay if I add one more test to the mix, for a missing value?
This is a patch showing the change to the diff (the extra test).

[Joseph Pecoraro]

The test I'm proposing is:

  7. <meta name="viewport" content="width=">
      ERROR: Viewport argument value "" for key "width" not recognized. Content ignored.

[Joseph Pecoraro]

Arg, the expected results for all of these tests are going to be blank.
That is because CONSOLE MESSAGES only make it to the DumpRenderTree
delegate if they are JSMessageSource messages and these messages
are HTMLMessageSource, because they come from parsing the HTML.

    if (source == JSMessageSource)
        page->chrome()->client()->addMessageToConsole(source, type, level, message, lineNumber, sourceURL);

I think its still worth keeping the tests. But it will be weird checking in
blank expected results. I can add private delegates, but that seems
like a lot of code that isn't used by anyone. I could also make these
manual tests. Any preferences?

[Joseph Pecoraro]

I just opened:
<http://webkit.org/b/54926> All Console Messages should be passed to ChromeClients.

I'll block on that, as these tests are useless without warnings in the expected results.

[Kenneth Rohde Christiansen]

(In reply to comment #4)
> Created an attachment (id=83231) [details]
> [PATCH] One more test for a missing value.
> 
> Kenneth, is it okay if I add one more test to the mix, for a missing value?
> This is a patch showing the change to the diff (the extra test).

Sure, please go ahead!

[Kenneth Rohde Christiansen]

(In reply to comment #7)
> I just opened:
> <http://webkit.org/b/54926> All Console Messages should be passed to ChromeClients.
> 
> I'll block on that, as these tests are useless without warnings in the expected results.

I think this would be the right thing to do

[Joseph Pecoraro]

Landed in r80068:
http://trac.webkit.org/changeset/80068

Watching the bots for expected results for viewport tests.

[Joseph Pecoraro]

Build fix for Qt in:
http://trac.webkit.org/changeset/80073

[Joseph Pecoraro]

Created attachment 84338 [details]
[FIX] Poor Build Fix solution for Qt WebKit2

Not up for review, since it was recommended I rollout.

[Joseph Pecoraro]

Rolled out r80068 and r80073 in r80077:
http://trac.webkit.org/changeset/80077

The fast/viewport tests will be reporting a lot of incorrect failures until
this is fixed. Any recommendations on WebKit2 Qt developers would
be appreciated. The [FIX] attachment I have above is a poor approach.
Maybe we should drop these tips altogether, the Document is only
used for tipping users about the "device-width" and "device-height"
constants. Removing these tips would simplify the patch.

[Joseph Pecoraro]

CC'ing some Qt developers to see if they have a suggestion.

[Csaba Osztrogonác]

Balazs, could you help fixing Qt-WK2 build?

[Balazs Kelemen]

(In reply to comment #13)
> Rolled out r80068 and r80073 in r80077:
> http://trac.webkit.org/changeset/80077
> 
> The fast/viewport tests will be reporting a lot of incorrect failures until
> this is fixed. Any recommendations on WebKit2 Qt developers would
> be appreciated. The [FIX] attachment I have above is a poor approach.
> Maybe we should drop these tips altogether, the Document is only
> used for tipping users about the "device-width" and "device-height"
> constants. Removing these tips would simplify the patch.

Currently there is no public qt-wk2 bot so I think the patch with the build
fix is good as it is if it not breaks tests with wk1.
We should find an way to fix this for wk2 or remove the tips as you suggested.

[Ryosuke Niwa]

Also see http://trac.webkit.org/changeset/80107 and http://trac.webkit.org/changeset/80114.

[David Kilzer (:ddkilzer)]

(In reply to comment #13)
> The fast/viewport tests will be reporting a lot of incorrect failures until
> this is fixed. Any recommendations on WebKit2 Qt developers would
> be appreciated. The [FIX] attachment I have above is a poor approach.
> Maybe we should drop these tips altogether, the Document is only
> used for tipping users about the "device-width" and "device-height"
> constants. Removing these tips would simplify the patch.

Another approach would be to turn off tips by default, but provide a way to enable them for individual tests so that we can still test them!

[Joseph Pecoraro]

Before I was just trying to match old behavior, but now that I am forced to
look deeper at this I think we can make things a little saner.

These tips are unique in that they need to know the deviceWidth and deviceHeight
known above WebCore by the WebKit clients. They also need a console to add the
tip too inside WebCore, but that is a more minor detail.

Currently the deviceWidth and deviceHeight are not known at parsing time,
but are known when computeViewportAttributes is called. In Qt by accessing
environmental variables like getintenv("QTWEBKIT_DEVICE_WIDTH"), and in
GTK by the width/height of the window rect accessed via
windowRect(webView->priv->corePage->chrome()->windowRect()).

I think the ideal solution is to have the deviceWidth and deviceHeight at the
time that we parse the viewport arguments. I suggest adding an accessor
on ChromeClient along the lines of:

  ChromeClient.h:
  virtual FloatRect viewportDeviceRect() = 0;

The advantages of this approach are that:

  • the tips are created at parse time, like all other viewport warnings/tips/errors
    so we are safe to report viewport warnings to the console like normal.
  • we eliminate the possibility that the warning could be printed multiple times
    because there is nothing that prevents computeViewportAttributes from be
    called/calculated multiple times.


While I'm at it, if we still want to keep this tip, I see there is a new keyword for
"desktop-width". While I haven't investigated this yet, we could also add an
accessor and a tip for this as well:

  ChromeClient.h:
  virtual float viewportDesktopWidth() = 0;

  New Tip when width constant matches viewportDesktopWidth():
  TIP: Viewport width constant matches desktop width, try using the "desktop-width" constant instead for future compatibility.


Finally, is the following really something we want to encourage?

  <meta name="viewport" content="width:device-height">

I was just thinking about matching old behavior, but I think we should only
tip about "device-width" for width, and "device-height" for height.

Do these suggestions sound good to you? I'll start a patch for the above, but
as you've seen I'm not the best with other ports =).

[Kenneth Rohde Christiansen]

(In reply to comment #19)
> Before I was just trying to match old behavior, but now that I am forced to
> look deeper at this I think we can make things a little saner.
> 
> These tips are unique in that they need to know the deviceWidth and deviceHeight
> known above WebCore by the WebKit clients. They also need a console to add the
> tip too inside WebCore, but that is a more minor detail.
> 
> Currently the deviceWidth and deviceHeight are not known at parsing time,
> but are known when computeViewportAttributes is called. In Qt by accessing
> environmental variables like getintenv("QTWEBKIT_DEVICE_WIDTH"), and in
> GTK by the width/height of the window rect accessed via
> windowRect(webView->priv->corePage->chrome()->windowRect()).

These are hardcoded in our webkit2 port though.

> I think the ideal solution is to have the deviceWidth and deviceHeight at the
> time that we parse the viewport arguments. I suggest adding an accessor
> on ChromeClient along the lines of:
> 
>   ChromeClient.h:
>   virtual FloatRect viewportDeviceRect() = 0;

Any reason why not just to use a FloatSize?

virtual FloatSize viewportDeviceSize() const = 0; (we want these to be const as well)

Also keep in mind that we are doing a DPI adjustment (with my device currently 1.5, but I guess iPhone 4 uses 2.0 due to the retina display). You will want an accessor for that as well

virtual float viewportDevicePixelRatio() const = 0.

The above is not the best solution, as we currently have one with similar name, which is used to tell the web content what dpi adjustment factor was used (@media all and (-webkit-pixel-ratio))

So it will be better with one like

virtual int viewportDeviceDPI() = 0;

> 
> The advantages of this approach are that:
> 
>   • the tips are created at parse time, like all other viewport warnings/tips/errors
>     so we are safe to report viewport warnings to the console like normal.
>   • we eliminate the possibility that the warning could be printed multiple times
>     because there is nothing that prevents computeViewportAttributes from be
>     called/calculated multiple times.
> 
> 
> While I'm at it, if we still want to keep this tip, I see there is a new keyword for
> "desktop-width". While I haven't investigated this yet, we could also add an
> accessor and a tip for this as well:

The desktop-width is what is used for laying out desktop pages (pages with no viewport).

The iPhone uses 980, IE on Windows Phone 7 uses 1024, and Android had 3 values to choose from (I believe the former plus 800 which I believe is default)

> 
>   ChromeClient.h:
>   virtual float viewportDesktopWidth() = 0;
> 
>   New Tip when width constant matches viewportDesktopWidth():
>   TIP: Viewport width constant matches desktop width, try using the "desktop-width" constant instead for future compatibility.
> 
> 
> Finally, is the following really something we want to encourage?
> 
>   <meta name="viewport" content="width:device-height">

Nope because that is not a valid viewport meta tag :-) I guess you want a = instead of :

Anyway, I'm not sure and it is currently supported, so why not?

> I was just thinking about matching old behavior, but I think we should only
> tip about "device-width" for width, and "device-height" for height.

I do not feel strong about this.

> Do these suggestions sound good to you? I'll start a patch for the above, but
> as you've seen I'm not the best with other ports =).

I think it sounds fine.

[Kenneth Rohde Christiansen]

I still would like it possible to make the layout tests use different device-width and -height for testing the actual algorithm

[Joseph Pecoraro]

> Any reason why not just to use a FloatSize?
> 
> virtual FloatSize viewportDeviceSize() const = 0; (we want these to be const as well)

Yes, that makes more sense and is what I intended.


> > Finally, is the following really something we want to encourage?
> > 
> >   <meta name="viewport" content="width:device-height">
> 
> Nope because that is not a valid viewport meta tag :-) I guess you want a = instead of :

Whoops, too much JavaScript lately. You're correct.

> Anyway, I'm not sure and it is currently supported, so why not?

It is supported, and should continue to be supported, but I'm not convinced
that it should be encouraged with a console tip. If someone writes width="480"
should we really say, try using "device-height" instead? I don't think so.

[Joseph Pecoraro]

again with the incorrect syntax... I meant "width=480"

[Joseph Pecoraro]

Created attachment 84525 [details]
[PATCH] For Try Bots

I didn't actually get much time to work on this today. This doesn't
handle WebKit2, it just has the default FloatSize() of 0x0. Lets see
how the bots do on this.

I noticed everyone seems to use IntSizes despite device-width and
height possibly being float values. I don't know why that is, but
I stuck with FloatSize for the time being and just casted appropriately.

[WebKit Review Bot]

Attachment 84525 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:77:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:532:  device_width is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:533:  device_height is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Attachment 84525 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8078538

[Collabora GTK+ EWS bot]

Attachment 84525 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/8078579

[Kenneth Rohde Christiansen]

Comment on attachment 84525 [details]
[PATCH] For Try Bots

View in context: https://bugs.webkit.org/attachment.cgi?id=84525&action=review

> Source/WebCore/dom/ViewportArguments.cpp:226
> +        FloatSize deviceSize = page->chrome()->client()->viewportDeviceSize();

In reality with our dpi adjustment, this should be something like FloatSize deviceSize = page->chrome()->client()->viewportDeviceSize() / pixelRatio;

For instance on the iPhone 4, if im not wrong, device-width will report 320, there in reality it is 640. ie, 640 / 2.0 = 320.

The Android extensions makes it possible to avoid this upscaling of the contents, by setting target-densitydpi = device-dpi, so we need to make sure that is parsed at this time.

[Joseph Pecoraro]

(In reply to comment #28)
> For instance on the iPhone 4, if im not wrong, device-width will
> report 320, there in reality it is 640. ie, 640 / 2.0 = 320.
> 
> The Android extensions makes it possible to avoid this upscaling of the
> contents, by setting target-densitydpi = device-dpi, so we need to make
> sure that is parsed at this time.

That is an interesting point. So, given that, we would need to wait until
both the target-densitydpi and width are parsed, so essentially after
the entire meta tag parsing? Is that really necessary.

  <meta name="viewport" content="width=320, target-densitydpi=2.0">
  <meta name="viewport" content="target-densitydpi=2.0, width=320">

With target-densitydpi does the "device-width" constant ever change,
or does it just change its effective value?

[Joseph Pecoraro]

style:
> Source/WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:77:  Alphabetical sorting problem.
> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:532:  device_width is incorrectly named. Don't use underscores in your identifier names.
> Source/WebKit/efl/WebCoreSupport/ChromeClientEfl.cpp:533:  device_height is incorrectly named. Don't use underscores in your identifier names.

Fixed. Updated the older efl style.

qt:
> Source/WebKit/qt/WebCoreSupport/ChromeClientQt.cpp: In function ‘QSize WebCore::queryDeviceSizeForScreenContainingWidget(const QWidget*)’:
> Source/WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:724: error: incomplete type ‘QApplication’ used in nested name specifier
> Source/WebKit/qt/WebCoreSupport/ChromeClientQt.cpp: In member function ‘virtual WebCore::FloatSize WebCore::ChromeClientQt::viewportDeviceSize() const’:
> Source/WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:759: error: ‘d’ was not declared in this scope

Fixed. Included qapplication and d is m_webPage->d.

efl:
> fatal: Unable to create '/mnt/eflews/git/webkit/.git/index.lock': File exists.

git issue.

gtk:

> Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.h:114: error: 'FloatSize' does not name a type
> Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:654: error: no 'WebCore::FloatSize WebKit::ChromeClient::viewportDeviceSize() const' member function declared in class 'WebKit::ChromeClient'

Fixed. Forgot namespace, so WebCore::FloatSize.

[Joseph Pecoraro]

Created attachment 84592 [details]
[PATCH] Try For Bots

Take 2.

[Kenneth Rohde Christiansen]

(In reply to comment #29)
> (In reply to comment #28)
> > For instance on the iPhone 4, if im not wrong, device-width will
> > report 320, there in reality it is 640. ie, 640 / 2.0 = 320.
> > 
> > The Android extensions makes it possible to avoid this upscaling of the
> > contents, by setting target-densitydpi = device-dpi, so we need to make
> > sure that is parsed at this time.
> 
> That is an interesting point. So, given that, we would need to wait until
> both the target-densitydpi and width are parsed, so essentially after
> the entire meta tag parsing? Is that really necessary.
> 
>   <meta name="viewport" content="width=320, target-densitydpi=2.0">
>   <meta name="viewport" content="target-densitydpi=2.0, width=320">
> 
> With target-densitydpi does the "device-width" constant ever change,
> or does it just change its effective value?

In the above case it is not necessary as you are using fixed values. But in the case of using device-width is it.

<meta name="viewport" content="width=device-width, target-densitydpi=160">

on a 480 wide device substitutes device-width with 320, due to the higher dpi of 240, where as 

<meta name="viewport" content="width=device-width, target-densitydpi=240">

substitutes device-width with 480.

The target-densitydpi only affects device-width and device-height substitution as well as sets the pixelRatio (target-densitydpi / 160; default is 160).

Google calls a pixel at 160 DPI a density independent pixel. The fact is that the original iPhone has a DPI of 160 which resulted in everyone adapting their web pages and web apps to that. On a phone with a higher DPI such as 240 the buttons etc become too small, so they are scaled up. Even the iPhone 4 scaled everything up with 2.0 when there is a viewport meta tag present.

With the target-densitydpi features, it is possible to actually take advantage of the increased DPI by for instance disabling the upscaling and using different CSS using media queries such as @media all and (-webkit-pixel-ratio... ) { }

[Joseph Pecoraro]

> In the above case it is not necessary as you are using fixed values.
> But in the case of using device-width is it.

If the user is already using the "device-width" keyword, we don't need
to provide a tip that they are using the keyword. The code change I'm
adding would never be reached in that case, only when there is a
numeric constant.


> <meta name="viewport" content="width=device-width, target-densitydpi=160">
> 
> on a 480 wide device substitutes device-width with 320, due to the higher dpi of 240, where as 
>
> <meta name="viewport" content="width=device-width, target-densitydpi=240">
> 
> substitutes device-width with 480.

Your descriptions sound backwards to the meta tags. Are they?

I see what you're saying, but in that case my question above still applies.
I'll rephrase it below with your examples (assuming they were backwards)
with the description.

On a 480px wide device, are you suggesting that both of following should
print a tip that the width constant could be device-width?

  <meta name="viewport" content="width=480, target-densitydpi=160">
  <meta name="viewport" content="width=320, target-densitydpi=240">

[Collabora GTK+ EWS bot]

Attachment 84592 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/8085334

[Joseph Pecoraro]

Created attachment 84617 [details]
[PATCH] For Try Bots

Take 3.

[Kenneth Rohde Christiansen]

(In reply to comment #33)
> > In the above case it is not necessary as you are using fixed values.
> > But in the case of using device-width is it.
> 
> If the user is already using the "device-width" keyword, we don't need
> to provide a tip that they are using the keyword. The code change I'm
> adding would never be reached in that case, only when there is a
> numeric constant.

Good point, and I guess we could do with that, but when I think about it

<meta name="viewport" content="width=320, target-densitydpi=160">

should yield a warning to use device-width, but so should

<meta name="viewport" content="width=480, target-densitydpi=240">
and
<meta name="viewport" content="width=480, target-densitydpi=device-dpi">

for a device with dpi 240 and device-width 480.

It all comes down to how useful these warnings are. You could say that if someone knows how to use target-densitydpi, he surely also knows how to use device-width. It could even be that the user actually wanted 480 always and just wanted the scale to change to fit the 480 into the given view. I have seem many such cases.

With that in mind, we could even show the warning always when someone uses a real number value instead of the device-* defines.

> > <meta name="viewport" content="width=device-width, target-densitydpi=160">
> > 
> > on a 480 wide device substitutes device-width with 320, due to the higher dpi of 240, where as 
> >
> > <meta name="viewport" content="width=device-width, target-densitydpi=240">
> > 
> > substitutes device-width with 480.
> 
> Your descriptions sound backwards to the meta tags. Are they?

Sorry, what do you mean?

> I see what you're saying, but in that case my question above still applies.
> I'll rephrase it below with your examples (assuming they were backwards)
> with the description.
> 
> On a 480px wide device, are you suggesting that both of following should
> print a tip that the width constant could be device-width?
> 
>   <meta name="viewport" content="width=480, target-densitydpi=160">
>   <meta name="viewport" content="width=320, target-densitydpi=240">

Yes, if 160 and 240 were swapped: :-)

device-width with target-densitydpi=240 on a 480px wide device is == 480, hense

   <meta name="viewport" content="width=480, target-densitydpi=240">
   <meta name="viewport" content="width=320, target-densitydpi=160">

[Kenneth Rohde Christiansen]

>    <meta name="viewport" content="width=480, target-densitydpi=240">
>    <meta name="viewport" content="width=320, target-densitydpi=160">

To explain myself better. By default we assume we are a 160 dpi device (as the original iPhone).

ie. device-width for a 160 dpi device is considered 320, which fits with the original iPhone.

But the device has a real width of 480 and a real dpi of 240. Hense using device-dpi or 240 should yield a device-width of 480, ie. it's actual width.

The calculo is: device-width = real width / (target-densitydpi / 160) = 480 / (240 / 160) = 480 / 1.5 = 320.

I hope that clears it up.

[Kenneth Rohde Christiansen]

> The calculo is: device-width = real width / (target-densitydpi / 160) = 480 / (240 / 160) = 480 / 1.5 = 320.

Strip that :-( I wrote it wrong

a) without target-densitydpi argument (assuming target-densitydpi of 160)

device-width = real width / (real dpi / target-densitydpi) = 480 / (240 / 160) = 480 / 1.5 = 320.

b) with target-densitydpi = device-dpi (which is 240)

device-width = real width / (real dpi / target-densitydpi) = 480 / (240 / 240) = 480 / 1.0 = 480.

[Joseph Pecoraro]

> a) without target-densitydpi argument (assuming target-densitydpi of 160)
> 
> device-width = real width / (real dpi / target-densitydpi) = 480 / (240 / 160) = 480 / 1.5 = 320.
> 
> b) with target-densitydpi = device-dpi (which is 240)
> 
> device-width = real width / (real dpi / target-densitydpi) = 480 / (240 / 240) = 480 / 1.0 = 480.

Okay, that is much clearer, Thanks!



> It all comes down to how useful these warnings are. You could say that if
> someone knows how to use target-densitydpi, he surely also knows how
> to use device-width. It could even be that the user actually wanted 480
> always and just wanted the scale to change to fit the 480 into the given
> view. I have seem many such cases.

In that case, it sounds to me like, with target-densitydpi in the mix, these
tips aren't as useful as they used to be. I'm fine with removing them. It
would greatly simplify the patch as well because I wouldn't need to touch
all the other ports.

However, if we want to improve the tip, what would be a good suggestion?
After Document::processViewport processes the ViewportArguments, we
could look at the end result and present some tips or warnings. I can't
think of any good ones off the top of my head.

[Joseph Pecoraro]

Created attachment 84657 [details]
[PATCH] Remove Confusing Tips - Improve Warnings + Errors

I decided to just remove them. Much simpler patch in the end.
After landing this I would need to rebase all viewport tests for
gtk/qt + add expected results for the new tests.

[Kenneth Rohde Christiansen]

Good work Joseph. I have a few viewport related patches that I will try upstreaming soonish. I will cc you to those.

[Joseph Pecoraro]

Landed in r80483:
http://trac.webkit.org/changeset/80483

I'll be watching the bots to pull expected test results for LayoutTests/fast/viewport/*.
Many are expected to change and some (added in this patch) will be new results.

[WebKit Review Bot]

http://trac.webkit.org/changeset/80483 might have broken Qt Linux Release
The following tests are not passing:
fast/viewport/viewport-100.html
fast/viewport/viewport-101.html
fast/viewport/viewport-102.html
fast/viewport/viewport-103.html
fast/viewport/viewport-104.html
fast/viewport/viewport-105.html
fast/viewport/viewport-106.html
fast/viewport/viewport-107.html
fast/viewport/viewport-108.html
fast/viewport/viewport-109.html
fast/viewport/viewport-110.html
fast/viewport/viewport-111.html
fast/viewport/viewport-112.html
fast/viewport/viewport-113.html
fast/viewport/viewport-114.html
fast/viewport/viewport-115.html
fast/viewport/viewport-116.html
fast/viewport/viewport-117.html
fast/viewport/viewport-118.html
fast/viewport/viewport-129.html
fast/viewport/viewport-29.html
fast/viewport/viewport-30.html
fast/viewport/viewport-31.html
fast/viewport/viewport-32.html
fast/viewport/viewport-35.html
fast/viewport/viewport-36.html
fast/viewport/viewport-38.html
fast/viewport/viewport-39.html
fast/viewport/viewport-40.html
fast/viewport/viewport-41.html
fast/viewport/viewport-42.html
fast/viewport/viewport-43.html
fast/viewport/viewport-44.html
fast/viewport/viewport-47.html
fast/viewport/viewport-48.html
fast/viewport/viewport-49.html
fast/viewport/viewport-61.html
fast/viewport/viewport-62.html
fast/viewport/viewport-63.html
fast/viewport/viewport-64.html
fast/viewport/viewport-66.html
fast/viewport/viewport-67.html
fast/viewport/viewport-68.html
fast/viewport/viewport-69.html
fast/viewport/viewport-70.html
fast/viewport/viewport-71.html
fast/viewport/viewport-72.html
fast/viewport/viewport-73.html
fast/viewport/viewport-74.html
fast/viewport/viewport-75.html
fast/viewport/viewport-76.html
fast/viewport/viewport-77.html
fast/viewport/viewport-78.html
fast/viewport/viewport-79.html
fast/viewport/viewport-80.html
fast/viewport/viewport-81.html
fast/viewport/viewport-83.html
fast/viewport/viewport-85.html
fast/viewport/viewport-88.html
fast/viewport/viewport-90.html

[Joseph Pecoraro]

Expected Results Landed in r80490:
http://trac.webkit.org/changeset/80490

I'll mark this as closed now, but keep an eye out for any outlying viewport
tests that other ports test but were not tested by the Qt port.