[Webkit-unassigned] [Bug 181716] Parse calc() in CSS media queries

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Jan 21 09:38:39 PST 2018


https://bugs.webkit.org/show_bug.cgi?id=181716

Antti Koivisto <koivisto at iki.fi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #331736|review?                     |review+
              Flags|                            |

--- Comment #35 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 331736
  --> https://bugs.webkit.org/attachment.cgi?id=331736
Patch

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

r=me

> Source/WebCore/css/MediaQueryExpression.cpp:185
> +    firstValue = CSSPropertyParserHelpers::consumeLength(range, HTMLStandardMode, ValueRangeNonNegative);
> +    if (firstValue)
> +        return firstValue;

These could be written somewhat more compactly as

if (auto value = CSSPropertyParserHelpers::consumeLength(range, HTMLStandardMode, ValueRangeNonNegative)) 
    return value;

> Source/WebCore/css/parser/MediaQueryParser.cpp:252
> +    if (m_state != ReadFeatureValue || type == WhitespaceToken) {
> +        handleBlocks(token);
> +        m_blockWatcher.handleToken(token);
> +        range.consume();
> +    }
>  
>      // Call the function that handles current state
>      if (type != WhitespaceToken)
> -        ((this)->*(m_state))(type, token);
> +        ((this)->*(m_state))(type, token, range);

I think a good refactoring would be to make State a normal enum class instead of a function pointer and replace this code with a switch (m_state) that simply invokes the right function. Then different cases could have different arguments (eliminating unused parameters and the no-op done() function) and the whole thing would be more readable.

That can be left for future patches though.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180121/ec373df7/attachment.html>


More information about the webkit-unassigned mailing list