Bug 229705 - Add support for CSSKeywordValue in CSS Typed OM
Summary: Add support for CSSKeywordValue in CSS Typed OM
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Qiaosong Zhou
URL:
Keywords: InRadar
Depends on:
Blocks: 175733
  Show dependency treegraph
 
Reported: 2021-08-31 03:09 PDT by Qiaosong Zhou
Modified: 2021-09-02 12:40 PDT (History)
18 users (show)

See Also:


Attachments
Patch (15.83 KB, patch)
2021-08-31 03:10 PDT, Qiaosong Zhou
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (16.34 KB, patch)
2021-08-31 03:33 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff
Patch (254.92 KB, patch)
2021-09-01 03:54 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff
Patch (254.86 KB, patch)
2021-09-01 03:57 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff
Patch (254.39 KB, patch)
2021-09-01 04:10 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff
Patch (254.39 KB, patch)
2021-09-01 15:55 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Qiaosong Zhou 2021-08-31 03:09:32 PDT
Add support for CSSKeywordValue in CSS Typed OM
Comment 1 Qiaosong Zhou 2021-08-31 03:10:00 PDT
Created attachment 436866 [details]
Patch
Comment 2 Qiaosong Zhou 2021-08-31 03:33:51 PDT
Created attachment 436869 [details]
Patch
Comment 3 Alex Christensen 2021-08-31 11:37:53 PDT
Comment on attachment 436869 [details]
Patch

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

This is a wonderful patch.

> Source/WebCore/css/typedom/CSSKeywordValue.cpp:2
> + * Copyright (C) 2019 Apple Inc. All rights reserved.

2021

> Source/WebCore/css/typedom/CSSKeywordValue.cpp:44
> +    if (value.isNull() || value.isEmpty())

value.isEmpty returns true if value is null, so only one check is needed.

> Source/WebCore/css/typedom/CSSKeywordValue.cpp:52
> +    if (value.isNull() || value.isEmpty())

ditto

> Source/WebCore/css/typedom/CSSKeywordValue.h:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

2021

> Source/WebCore/css/typedom/CSSKeywordValue.h:30
> +#include "CSSKeywordValue.h"

This shouldn't include itself.

> Source/WebCore/css/typedom/CSSKeywordValue.h:32
> +#include <wtf/RefCounted.h>

This doesn't look needed.

> Source/WebCore/css/typedom/CSSKeywordValue.h:44
> +    String& value() { return m_value; }

const String& value() const

> Source/WebCore/css/typedom/CSSKeywordValue.idl:2
> +* Copyright (C) 2018 Apple Inc.  All rights reserved.

2021
Comment 4 Qiaosong Zhou 2021-09-01 03:54:53 PDT
Created attachment 437010 [details]
Patch
Comment 5 Qiaosong Zhou 2021-09-01 03:57:07 PDT
Created attachment 437011 [details]
Patch
Comment 6 Qiaosong Zhou 2021-09-01 04:10:58 PDT
Created attachment 437012 [details]
Patch
Comment 7 Alex Christensen 2021-09-01 09:31:33 PDT
Comment on attachment 437012 [details]
Patch

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

And three copyright years need updating, but otherwise this looks ready to go.

> Source/WebCore/css/typedom/CSSKeywordValue.h:43
> +    const String& value() { return m_value; }

This needs a const between () and { to indicate that calling the function doesn't mutate the object.  The first const indicates that you can't mutate the returned reference.
Comment 8 Qiaosong Zhou 2021-09-01 15:55:16 PDT
Created attachment 437075 [details]
Patch
Comment 9 EWS 2021-09-02 12:39:12 PDT
Committed r281947 (241255@main): <https://commits.webkit.org/241255@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437075 [details].
Comment 10 Radar WebKit Bug Importer 2021-09-02 12:40:33 PDT
<rdar://problem/82688747>