Changeset 148186 in webkit


Ignore:
Timestamp:
Apr 11, 2013 2:00:22 AM (11 years ago)
Author:
commit-queue@webkit.org
Message:

Incorrect evaluation of resolution media queries
​https://bugs.webkit.org/show_bug.cgi?id=114029

Patch by Rune Lillesveen <​rune@opera.com> on 2013-04-11
Reviewed by Kenneth Rohde Christiansen.

.:

Removed setResolutionOverride from exports.

  • Source/autotools/symbols.filter:

Source/WebCore:

The implementation used the physical resolution to evaluate the
resolution media features. Changed to use the actual CSS resolution,
also known as the device-pixel-ratio, instead. Unified the code for
evaluating the resolution and device-pixel-ratio media features.

No new tests, covered by existing tests.

  • WebCore.exp.in:
  • css/CSSPrimitiveValue.h:

(WebCore::CSSPrimitiveValue::isResolution):

  • css/MediaQueryEvaluator.cpp:

(WebCore::evalResolution):
(WebCore::device_pixel_ratioMediaFeatureEval):
(WebCore::resolutionMediaFeatureEval):

  • page/Screen.cpp:
  • page/Screen.h:
  • page/Settings.cpp:

(WebCore):

  • page/Settings.h:

(Settings):

  • testing/InternalSettings.cpp:

(WebCore::InternalSettings::Backup::Backup):
(WebCore::InternalSettings::Backup::restoreTo):

  • testing/InternalSettings.h:

(Backup):
(InternalSettings):

  • testing/InternalSettings.idl:

Source/WebKit:

Removed setResolutionOverride from exports.

  • WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in:

Source/WebKit/win:

Removed setResolutionOverride from exports.

  • WebKit.vcproj/WebKitExports.def.in:

LayoutTests:

Modified tests to change CSS resolution instead of physical resolution.

  • fast/media/mq-resolution.html:
Location:
trunk
Files:
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/ChangeLog

    r148088 r148186  
     12013-04-11  Rune Lillesveen  <rune@opera.com>
     2
     3        Incorrect evaluation of resolution media queries
     4        https://bugs.webkit.org/show_bug.cgi?id=114029
     5
     6        Reviewed by Kenneth Rohde Christiansen.
     7
     8        Removed setResolutionOverride from exports.
     9
     10        * Source/autotools/symbols.filter:
     11
    1122013-04-10  Anton Obzhirov  <a.obzhirov@samsung.com>
    213

  • trunk/LayoutTests/ChangeLog

    r148182 r148186  
     12013-04-11  Rune Lillesveen  <rune@opera.com>
     2
     3        Incorrect evaluation of resolution media queries
     4        https://bugs.webkit.org/show_bug.cgi?id=114029
     5
     6        Reviewed by Kenneth Rohde Christiansen.
     7
     8        Modified tests to change CSS resolution instead of physical resolution.
     9
     10        * fast/media/mq-resolution.html:
     11
    1122013-04-11  Manuel Rego Casasnovas  <rego@igalia.com>
    213

  • trunk/LayoutTests/fast/media/mq-resolution.html

    r132490 r148186  
    5050        shouldBe("matchMedia('(max-resolution: 0dpi)').matches", "false");
    5151
    52         window.internals.settings.setResolutionOverride(144, 144);
     52        window.internals.settings.setPageScaleFactor(1.5, 0, 0);
    5353        shouldBe("matchMedia('(resolution: 1.5dppx)').matches", "true");
    5454        shouldBe("resolutionFromStyle()", "1.5");
    5555
    56         window.internals.settings.setResolutionOverride(192, 192);
     56        window.internals.settings.setPageScaleFactor(2, 0, 0);
    5757        shouldBe("matchMedia('(resolution: 2dppx)').matches", "true");
    5858        shouldBe("resolutionFromStyle()", "2");
    5959
    60         window.internals.settings.setResolutionOverride(96, 96);
     60        window.internals.settings.setPageScaleFactor(1, 0, 0);
    6161        shouldBe("matchMedia('(resolution: 1dppx)').matches", "true");
    6262        shouldBe("resolutionFromStyle()", "1");
    6363
    64         window.internals.settings.setResolutionOverride(216, 216);
     64        window.internals.settings.setPageScaleFactor(2.25, 0, 0);
    6565        shouldBe("matchMedia('(resolution: 2.25dppx)').matches", "true");
    6666        shouldBe("resolutionFromStyle()", "2.25");
    … …  
    7373        shouldBe("matchMedia('(min-resolution: 85dpcm)').matches", "true");
    7474        shouldBe("matchMedia('(max-resolution: 85dpcm)').matches", "true");
    75 
    76         window.internals.settings.setResolutionOverride(216, 254);
    77         // Non-square must never match.
    78         shouldBe("matchMedia('(resolution)').matches", "false");
    79         shouldBe("matchMedia('(resolution: 216dpi)').matches", "false");
    80         shouldBe("matchMedia('(resolution: 254dpi)').matches", "false");
    81 
    82         shouldBe("matchMedia('(min-resolution: 216dpi)').matches", "true");
    83         shouldBe("matchMedia('(min-resolution: 254dpi)').matches", "false");
    84         shouldBe("matchMedia('(max-resolution: 216dpi)').matches", "false");
    85         shouldBe("matchMedia('(max-resolution: 254dpi)').matches", "true");
    86 
    87         // Non-square must never match.
    88         shouldBe("matchMedia('(resolution: 85dpcm)').matches", "false");
    89         shouldBe("matchMedia('(resolution: 100dpcm)').matches", "false");
    90 
    91         shouldBe("matchMedia('(min-resolution: 85dpcm)').matches", "true");
    92         shouldBe("matchMedia('(min-resolution: 100dpcm)').matches", "false");
    93         shouldBe("matchMedia('(max-resolution: 85dpcm)').matches", "false");
    94         shouldBe("matchMedia('(max-resolution: 100dpcm)').matches", "true");
    9575
    9676        // Test printing.

  • trunk/Source/WebCore/ChangeLog

    r148182 r148186  
     12013-04-11  Rune Lillesveen  <rune@opera.com>
     2
     3        Incorrect evaluation of resolution media queries
     4        https://bugs.webkit.org/show_bug.cgi?id=114029
     5
     6        Reviewed by Kenneth Rohde Christiansen.
     7
     8        The implementation used the physical resolution to evaluate the
     9        resolution media features. Changed to use the actual CSS resolution,
     10        also known as the device-pixel-ratio, instead. Unified the code for
     11        evaluating the resolution and device-pixel-ratio media features.
     12
     13        No new tests, covered by existing tests.
     14
     15        * WebCore.exp.in:
     16        * css/CSSPrimitiveValue.h:
     17        (WebCore::CSSPrimitiveValue::isResolution):
     18        * css/MediaQueryEvaluator.cpp:
     19        (WebCore::evalResolution):
     20        (WebCore::device_pixel_ratioMediaFeatureEval):
     21        (WebCore::resolutionMediaFeatureEval):
     22        * page/Screen.cpp:
     23        * page/Screen.h:
     24        * page/Settings.cpp:
     25        (WebCore):
     26        * page/Settings.h:
     27        (Settings):
     28        * testing/InternalSettings.cpp:
     29        (WebCore::InternalSettings::Backup::Backup):
     30        (WebCore::InternalSettings::Backup::restoreTo):
     31        * testing/InternalSettings.h:
     32        (Backup):
     33        (InternalSettings):
     34        * testing/InternalSettings.idl:
     35
    1362013-04-11  Carlos Garcia Campos  <cgarcia@igalia.com>
    237

  • trunk/Source/WebCore/WebCore.exp.in

    r148137 r148186  
    10651065__ZN7WebCore8Settings42setHiddenPageCSSAnimationSuspensionEnabledEb
    10661066__ZN7WebCore8Settings45setShouldRespectPriorityInCSSAttributeSettersEb
    1067 __ZN7WebCore8Settings21setResolutionOverrideERKNS_7IntSizeE
    10681067__ZN7WebCore8Settings20setMediaTypeOverrideERKN3WTF6StringE
    10691068__ZN7WebCore8Settings30setShowTiledScrollingIndicatorEb

  • trunk/Source/WebCore/css/CSSPrimitiveValue.h

    r142904 r148186  
    190190    bool isDotsPerPixel() const { return primitiveType() == CSS_DPPX; }
    191191    bool isDotsPerCentimeter() const { return primitiveType() == CSS_DPCM; }
     192    bool isResolution() const
     193    {
     194        unsigned short type = primitiveType();
     195        return type >= CSS_DPPX && type <= CSS_DPCM;
     196    }
    192197
    193198#if ENABLE(CSS_VARIABLES)

  • trunk/Source/WebCore/css/MediaQueryEvaluator.cpp

    r145255 r148186  
    199199}
    200200
    201 #if ENABLE(RESOLUTION_MEDIA_QUERY)
    202 static bool compareResolution(float min, float max, float value, MediaFeaturePrefix op)
    203 {
    204     switch (op) {
    205     case NoPrefix:
    206         // A 'resolution' (without a "min-" or "max-" prefix) query
    207         // never matches a device with non-square pixels.
    208         return value == min && value == max;
    209     case MinPrefix:
    210         return min >= value;
    211     case MaxPrefix:
    212         return max <= value;
    213     }
    214     return false;
    215 }
    216 #endif
    217 
    218201static bool numberValue(CSSValue* value, float& result)
    219202{
    … …  
    289272}
    290273
    291 static bool device_pixel_ratioMediaFeatureEval(CSSValue *value, RenderStyle*, Frame* frame, MediaFeaturePrefix op)
     274static bool evalResolution(CSSValue* value, Frame* frame, MediaFeaturePrefix op)
    292275{
    293276    // FIXME: Possible handle other media types than 'screen' and 'print'.
    … …  
    311294        return !!deviceScaleFactor;
    312295
    313     return value->isPrimitiveValue() && compareValue(deviceScaleFactor, static_cast<CSSPrimitiveValue*>(value)->getFloatValue(), op);
    314 }
    315 
    316 static bool resolutionMediaFeatureEval(CSSValue* value, RenderStyle*, Frame* frame, MediaFeaturePrefix op)
    317 {
    318 #if ENABLE(RESOLUTION_MEDIA_QUERY)
    319     // The DPI below is dots per CSS inch and thus not device inch. The
    320     // functions should respect this.
    321     //
    322     // For square pixels, it is simply the device scale factor (dppx) times 96,
    323     // per definition.
    324     //
    325     // The device scale factor is a predefined value which is calculated per
    326     // device given the preferred distance in arms length (considered one arms
    327     // length for desktop computers and usually 0.6 arms length for phones).
    328     //
    329     // The value can be calculated as follows (rounded to quarters):
    330     //     round((deviceDotsPerInch * distanceInArmsLength / 96) * 4) / 4.
    331     // Example (mid-range resolution phone):
    332     //     round((244 * 0.6 / 96) * 4) / 4 = 1.5
    333     // Example (high-range resolution laptop):
    334     //     round((220 * 1.0 / 96) * 4) / 4 = 2.0
    335 
    336     float horiDPI;
    337     float vertDPI;
    338 
    339     // This checks the actual media type applied to the document, and we know
    340     // this method only got called if this media type matches the one defined
    341     // in the query. Thus, if if the document's media type is "print", the
    342     // media type of the query will either be "print" or "all".
    343     String mediaType = frame->view()->mediaType();
    344     if (equalIgnoringCase(mediaType, "screen")) {
    345         Screen* screen = frame->document()->domWindow()->screen();
    346         horiDPI = screen->horizontalDPI();
    347         vertDPI = screen->verticalDPI();
    348     } else if (equalIgnoringCase(mediaType, "print")) {
    349         // The resolution of images while printing should not depend on the dpi
    350         // of the screen. Until we support proper ways of querying this info
    351         // we use 300px which is considered minimum for current printers.
    352         horiDPI = vertDPI = 300;
    353     } else {
    354         // FIXME: Possible handle other media types than 'screen' and 'print'.
    355         // For now, do not match.
    356         return false;
    357     }
    358 
    359     float leastDenseDPI = std::min(horiDPI, vertDPI);
    360     float mostDenseDPI = std::max(horiDPI, vertDPI);
    361 
    362     // According to spec, (resolution) will evaluate to true if (resolution:x)
    363     // will evaluate to true for a value x other than zero or zero followed by
    364     // a valid unit identifier (i.e., other than 0, 0dpi, 0dpcm, or 0dppx.),
    365     // which is always the case. But the spec special cases 'resolution' to
    366     // never matches a device with non-square pixels.
    367     if (!value) {
    368         ASSERT(op == NoPrefix);
    369         return leastDenseDPI == mostDenseDPI;
    370     }
    371 
    372296    if (!value->isPrimitiveValue())
    373297        return false;
    374298
    375     // http://dev.w3.org/csswg/css3-values/#resolution defines resolution as a
    376     // dimension, which contains a number (decimal point allowed), not just an
    377     // integer. Also, http://dev.w3.org/csswg/css3-values/#numeric-types says
    378     // "CSS theoretically supports infinite precision and infinite ranges for
    379     // all value types;
    380     CSSPrimitiveValue* rawValue = static_cast<CSSPrimitiveValue*>(value);
    381 
    382     if (rawValue->isDotsPerPixel()) {
    383         // http://dev.w3.org/csswg/css3-values/#absolute-lengths recommends
    384         // "that the pixel unit refer to the whole number of device pixels that
    385         // best approximates the reference pixel". We compare with 3 decimal
    386         // points, which aligns with current device-pixel-ratio's in use.
    387         float leastDenseDensity = floorf(leastDenseDPI * 1000 / 96) / 1000;
    388         float mostDenseDensity = floorf(leastDenseDPI * 1000 / 96) / 1000;
    389         float testedDensity = rawValue->getFloatValue(CSSPrimitiveValue::CSS_DPPX);
    390         return compareResolution(leastDenseDensity, mostDenseDensity, testedDensity, op);
    391     }
    392 
    393     if (rawValue->isDotsPerInch()) {
    394         unsigned testedDensity = rawValue->getFloatValue(CSSPrimitiveValue::CSS_DPI);
    395         return compareResolution(leastDenseDPI, mostDenseDPI, testedDensity, op);
    396     }
    397 
    398     // http://dev.w3.org/csswg/css3-values/#absolute-lengths recommends "that
    399     // the pixel unit refer to the whole number of device pixels that best
    400     // approximates the reference pixel".
    401     float leastDenseDPCM = roundf(leastDenseDPI / 2.54); // (2.54 cm/in)
    402     float mostDenseDPCM = roundf(mostDenseDPI / 2.54);
    403 
    404     if (rawValue->isDotsPerCentimeter()) {
    405         float testedDensity = rawValue->getFloatValue(CSSPrimitiveValue::CSS_DPCM);
    406         return compareResolution(leastDenseDPCM, mostDenseDPCM, testedDensity, op);
    407     }
     299    CSSPrimitiveValue* resolution = static_cast<CSSPrimitiveValue*>(value);
     300    return compareValue(deviceScaleFactor, resolution->isNumber() ? resolution->getFloatValue() : resolution->getFloatValue(CSSPrimitiveValue::CSS_DPPX), op);
     301}
     302
     303static bool device_pixel_ratioMediaFeatureEval(CSSValue *value, RenderStyle*, Frame* frame, MediaFeaturePrefix op)
     304{
     305    return (!value || static_cast<CSSPrimitiveValue*>(value)->isNumber()) && evalResolution(value, frame, op);
     306}
     307
     308static bool resolutionMediaFeatureEval(CSSValue* value, RenderStyle*, Frame* frame, MediaFeaturePrefix op)
     309{
     310#if ENABLE(RESOLUTION_MEDIA_QUERY)
     311    return (!value || static_cast<CSSPrimitiveValue*>(value)->isResolution()) && evalResolution(value, frame, op);
    408312#else
    409313    UNUSED_PARAM(value);
    410314    UNUSED_PARAM(frame);
    411315    UNUSED_PARAM(op);
     316    return false;
    412317#endif
    413 
    414     return false;
    415318}
    416319

  • trunk/Source/WebCore/page/Screen.cpp

    r135165 r148186  
    4444    : DOMWindowProperty(frame)
    4545{
    46 }
    47 
    48 unsigned Screen::horizontalDPI() const
    49 {
    50     if (!m_frame)
    51         return 0;
    52 
    53     // Used by the testing system, can be set from internals.
    54     IntSize override = m_frame->page()->settings()->resolutionOverride();
    55     if (!override.isEmpty())
    56         return override.width();
    57 
    58     // The DPI is defined as dots per CSS inch and thus not device inch.
    59     return m_frame->page()->deviceScaleFactor() * 96;
    60 }
    61 
    62 unsigned Screen::verticalDPI() const
    63 {
    64     // The DPI is defined as dots per CSS inch and thus not device inch.
    65     if (!m_frame)
    66         return 0;
    67 
    68     // Used by the testing system, can be set from internals.
    69     IntSize override = m_frame->page()->settings()->resolutionOverride();
    70     if (!override.isEmpty())
    71         return override.height();
    72 
    73     // The DPI is defined as dots per CSS inch and thus not device inch.
    74     return m_frame->page()->deviceScaleFactor() * 96;
    7546}
    7647

  • trunk/Source/WebCore/page/Screen.h

    r135058 r148186  
    4444        static PassRefPtr<Screen> create(Frame *frame) { return adoptRef(new Screen(frame)); }
    4545
    46         unsigned horizontalDPI() const;
    47         unsigned verticalDPI() const;
    4846        unsigned height() const;
    4947        unsigned width() const;

  • trunk/Source/WebCore/page/Settings.cpp

    r147893 r148186  
    321321#endif
    322322
    323 void Settings::setResolutionOverride(const IntSize& densityPerInchOverride)
    324 {
    325     if (m_resolutionDensityPerInchOverride == densityPerInchOverride)
    326         return;
    327 
    328     m_resolutionDensityPerInchOverride = densityPerInchOverride;
    329     m_page->setNeedsRecalcStyleInAllFrames();
    330 }
    331 
    332323void Settings::setMediaTypeOverride(const String& mediaTypeOverride)
    333324{

  • trunk/Source/WebCore/page/Settings.h

    r147795 r148186  
    108108
    109109        // Only set by Layout Tests.
    110         void setResolutionOverride(const IntSize&);
    111         const IntSize& resolutionOverride() const { return m_resolutionDensityPerInchOverride; }
    112 
    113         // Only set by Layout Tests.
    114110        void setMediaTypeOverride(const String&);
    115111        const String& mediaTypeOverride() const { return m_mediaTypeOverride; }
    … …  
    293289        bool m_textAutosizingEnabled : 1;
    294290#endif
    295         IntSize m_resolutionDensityPerInchOverride;
    296291
    297292        SETTINGS_MEMBER_VARIABLES

  • trunk/Source/WebCore/testing/InternalSettings.cpp

    r147893 r148186  
    8282    , m_originalTextAutosizingFontScaleFactor(settings->textAutosizingFontScaleFactor())
    8383#endif
    84     , m_originalResolutionOverride(settings->resolutionOverride())
    8584    , m_originalMediaTypeOverride(settings->mediaTypeOverride())
    8685#if ENABLE(DIALOG_ELEMENT)
    … …  
    120119    settings->setTextAutosizingFontScaleFactor(m_originalTextAutosizingFontScaleFactor);
    121120#endif
    122     settings->setResolutionOverride(m_originalResolutionOverride);
    123121    settings->setMediaTypeOverride(m_originalMediaTypeOverride);
    124122#if ENABLE(DIALOG_ELEMENT)
    … …  
    331329}
    332330
    333 void InternalSettings::setResolutionOverride(int dotsPerCSSInchHorizontally, int dotsPerCSSInchVertically, ExceptionCode& ec)
    334 {
    335     InternalSettingsGuardForSettings();
    336     // An empty size resets the override.
    337     settings()->setResolutionOverride(IntSize(dotsPerCSSInchHorizontally, dotsPerCSSInchVertically));
    338 }
    339 
    340331void InternalSettings::setMediaTypeOverride(const String& mediaType, ExceptionCode& ec)
    341332{

  • trunk/Source/WebCore/testing/InternalSettings.h

    r147034 r148186  
    6666        float m_originalTextAutosizingFontScaleFactor;
    6767#endif
    68         IntSize m_originalResolutionOverride;
    6968        String m_originalMediaTypeOverride;
    7069#if ENABLE(DIALOG_ELEMENT)
    … …  
    113112    void setTextAutosizingWindowSizeOverride(int width, int height, ExceptionCode&);
    114113    void setTextAutosizingFontScaleFactor(float fontScaleFactor, ExceptionCode&);
    115     void setResolutionOverride(int dotsPerCSSInchHorizontally, int dotsPerCSSInchVertically, ExceptionCode&);
    116114    void setMediaTypeOverride(const String& mediaType, ExceptionCode&);
    117115    void setCSSExclusionsEnabled(bool enabled, ExceptionCode&);

  • trunk/Source/WebCore/testing/InternalSettings.idl

    r147034 r148186  
    4343    void setTextAutosizingWindowSizeOverride(in long width, in long height) raises(DOMException);
    4444    void setTextAutosizingFontScaleFactor(in float fontScaleFactor) raises(DOMException);
    45     void setResolutionOverride(in long dotsPerCSSInchHorizontally, in long dotsPerCSSInchVertically) raises(DOMException);
    4645    void setMediaTypeOverride(in DOMString mediaTypeOverride) raises(DOMException);
    4746    void setCSSExclusionsEnabled(in boolean enabled) raises(DOMException);

  • trunk/Source/WebKit/ChangeLog

    r147974 r148186  
     12013-04-11  Rune Lillesveen  <rune@opera.com>
     2
     3        Incorrect evaluation of resolution media queries
     4        https://bugs.webkit.org/show_bug.cgi?id=114029
     5
     6        Reviewed by Kenneth Rohde Christiansen.
     7
     8        Removed setResolutionOverride from exports.
     9
     10        * WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in:
     11
    1122013-04-05  Roger Fong  <roger_fong@apple.com>
    213

  • trunk/Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in

    r147974 r148186  
    275275        ?setEditingValue@HTMLInputElement@WebCore@@QAEXABVString@WTF@@@Z
    276276        ?setPseudo@Element@WebCore@@QAEXABVAtomicString@WTF@@@Z
    277         ?setResolutionOverride@Settings@WebCore@@QAEXABVIntSize@2@@Z
    278277        ?setMediaTypeOverride@Settings@WebCore@@QAEXABVString@WTF@@@Z
    279278        ?settings@Document@WebCore@@QBEPAVSettings@2@XZ

  • trunk/Source/WebKit/win/ChangeLog

    r148123 r148186  
     12013-04-11  Rune Lillesveen  <rune@opera.com>
     2
     3        Incorrect evaluation of resolution media queries
     4        https://bugs.webkit.org/show_bug.cgi?id=114029
     5
     6        Reviewed by Kenneth Rohde Christiansen.
     7
     8        Removed setResolutionOverride from exports.
     9
     10        * WebKit.vcproj/WebKitExports.def.in:
     11
    1122013-04-08  Anders Carlsson  <andersca@apple.com>
    213

  • trunk/Source/WebKit/win/WebKit.vcproj/WebKitExports.def.in

    r147974 r148186  
    275275        ?setEditingValue@HTMLInputElement@WebCore@@QAEXABVString@WTF@@@Z
    276276        ?setPseudo@Element@WebCore@@QAEXABVAtomicString@WTF@@@Z
    277         ?setResolutionOverride@Settings@WebCore@@QAEXABVIntSize@2@@Z
    278277        ?setMediaTypeOverride@Settings@WebCore@@QAEXABVString@WTF@@@Z
    279278        ?settings@Document@WebCore@@QBEPAVSettings@2@XZ

  • trunk/Source/autotools/symbols.filter

    r147997 r148186  
    218218_ZN7WebCore8Settings20setMediaTypeOverrideERKN3WTF6StringE;
    219219_ZN7WebCore8Settings21mockScrollbarsEnabledEv;
    220 _ZN7WebCore8Settings21setResolutionOverrideERKNS_7IntSizeE;
    221220_ZN7WebCore8Settings21setShowRepaintCounterEb;
    222221_ZN7WebCore8Settings21setStandardFontFamilyERKN3WTF12AtomicStringE11UScriptCode;
Note: See TracChangeset for help on using the changeset viewer.