Bug 140056 – Modernize and streamline HTMLStackItem

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Darin Adler]

Modernize and streamline HTMLStackItem

[Darin Adler]

Created attachment 243936 [details]
Patch

[Build Bot]

Comment on attachment 243936 [details]
Patch

Attachment 243936 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5627747011395584

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 243938 [details]
Archive of layout-test-results from ews102 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 243936 [details]
Patch

Attachment 243936 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4856332632981504

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 243940 [details]
Archive of layout-test-results from ews105 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Darin Adler]

Created attachment 243948 [details]
Patch

[Darin Adler]

Created attachment 243967 [details]
Patch

[Anders Carlsson]

Comment on attachment 243967 [details]
Patch

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

> Source/WebCore/html/parser/HTMLStackItem.h:3
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Nice.

> Source/WebCore/html/parser/HTMLStackItem.h:42
> +    static Ref<HTMLStackItem> create(Ref<Element>, AtomicHTMLToken&, const AtomicString& namespaceURI = HTMLNames::xhtmlNamespaceURI);

Shouldn't Ref<Element> be Ref<Element>&& here?

> Source/WebCore/html/parser/HTMLStackItem.h:64
> +    HTMLStackItem(Ref<Element>, AtomicHTMLToken&, const AtomicString& namespaceURI);

And here.

[Darin Adler]

Committed r177910: <http://trac.webkit.org/changeset/177910>

[Darin Adler]

Comment on attachment 243967 [details]
Patch

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

>> Source/WebCore/html/parser/HTMLStackItem.h:42
>> +    static Ref<HTMLStackItem> create(Ref<Element>, AtomicHTMLToken&, const AtomicString& namespaceURI = HTMLNames::xhtmlNamespaceURI);
> 
> Shouldn't Ref<Element> be Ref<Element>&& here?

I guess so. Sorry missed that, so I’ll do it in a follow up.

>> Source/WebCore/html/parser/HTMLStackItem.h:64
>> +    HTMLStackItem(Ref<Element>, AtomicHTMLToken&, const AtomicString& namespaceURI);
> 
> And here.

Will do.

[David Kilzer (:ddkilzer)]

(In reply to comment #9)
> Committed r177910: <http://trac.webkit.org/changeset/177910>

This seems to have broken trunk iOS builds as well:

Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2525:111: error: indirection requires pointer operand ('WebCore::ContainerNode' invalid)
    if (!isParsingFragment() && m_tree.isTelephoneNumberParsingEnabled() && shouldParseTelephoneNumbersInNode(*m_tree.currentNode()) && TelephoneNumberDetector::isSupported())
                                                                                                              ^~~~~~~~~~~~~~~~~~~~~
1 error generated.

[David Kilzer (:ddkilzer)]

(In reply to comment #11)
> (In reply to comment #9)
> > Committed r177910: <http://trac.webkit.org/changeset/177910>
> 
> This seems to have broken trunk iOS builds as well:
> 
> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2525:111: error: indirection
> requires pointer operand ('WebCore::ContainerNode' invalid)
>     if (!isParsingFragment() && m_tree.isTelephoneNumberParsingEnabled() &&
> shouldParseTelephoneNumbersInNode(*m_tree.currentNode()) &&
> TelephoneNumberDetector::isSupported())
>                                                                             
> ^~~~~~~~~~~~~~~~~~~~~
> 1 error generated.

Attempted fix by Dan Bernstein:  http://trac.webkit.org/changeset/177916