Bug 62922 – indexForVisiblePosition(const VisiblePosition& visiblePosition) does not consider shadow content

• Component: HTML Editing
• Status: RESOLVED FIXED

[Justin Garcia]

indexForVisiblePosition(const VisiblePosition& visiblePosition) does not visiblePositions in shadow content.

[Justin Garcia]

Created attachment 98991 [details]
patch

[WebKit Review Bot]

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

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

Source/WebCore/editing/InsertListCommand.cpp:155:  Declaration has space between type name and * in Element *scope  [whitespace/declaration] [3]
Source/WebCore/editing/InsertListCommand.cpp:155:  Use 0 instead of NULL.  [readability/null] [5]
Source/WebCore/editing/htmlediting.cpp:1027:  Declaration has space between type name and * in Document *document  [whitespace/declaration] [3]
Source/WebCore/editing/htmlediting.cpp:1029:  Declaration has space between type name and * in Element *root  [whitespace/declaration] [3]
Source/WebCore/editing/htmlediting.cpp:1030:  Declaration has space between type name and * in Node *shadowRoot  [whitespace/declaration] [3]
Source/WebCore/editing/ApplyBlockElementCommand.cpp:84:  Declaration has space between type name and * in Element *startScope  [whitespace/declaration] [3]
Source/WebCore/editing/ApplyBlockElementCommand.cpp:84:  Use 0 instead of NULL.  [readability/null] [5]
Source/WebCore/editing/ApplyBlockElementCommand.cpp:86:  Declaration has space between type name and * in Element *endScope  [whitespace/declaration] [3]
Source/WebCore/editing/ApplyBlockElementCommand.cpp:86:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 9 in 10 files


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

[Justin Garcia]

Comment on attachment 98991 [details]
patch

Clearing review flag to fix style issues.

[Justin Garcia]

Created attachment 99018 [details]
updated patch

[Andy Estes]

Comment on attachment 99018 [details]
updated patch

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

I have a few style comments.

> WebCore/editing/htmlediting.cpp:1035
> +        root = static_cast<Element *>(shadowRoot);

This should be static_cast<Element*>() per the style guide.

> WebCore/editing/htmlediting.cpp:1047
> +    RefPtr<Range> range = Range::create(document, 
> +                                        firstPositionInNode(root),
>                                          p.parentAnchoredEquivalent());
> +    

The last two lines should be regularly indented, not lined up with the '('.

> WebCore/editing/htmlediting.cpp:1054
> +    // indexForVisiblePosition can create a

Can create a what?

[Justin Garcia]

Created attachment 99043 [details]
patch

Updated the patch to address the problems Andy pointed out.

[Enrica Casucci]

Comment on attachment 99043 [details]
patch

Looks good to me.

[Justin Garcia]

http://trac.webkit.org/changeset/90072