Implement <attachment> element appearance on Mac

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

Implement <attachment> element appearance on Mac

[Tim Horton]

Created attachment 247338 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  The parameter name "allocator" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  The parameter name "URL" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  _LSBindingCreateWithURL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 11 files


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

[Build Bot]

Comment on attachment 247338 [details]
Patch

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

New failing tests:
fast/attachment/attachment-rendering.html

[Build Bot]

Created attachment 247345 [details]
Archive of layout-test-results from ews101 for mac-mavericks

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

[Build Bot]

Comment on attachment 247338 [details]
Patch

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

New failing tests:
fast/attachment/attachment-rendering.html

[Build Bot]

Created attachment 247348 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

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

[Tim Horton]

Created attachment 247354 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  The parameter name "allocator" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  The parameter name "URL" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  _LSBindingCreateWithURL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 15 files


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

[Tim Horton]

Created attachment 247376 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  The parameter name "allocator" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  The parameter name "URL" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  _LSBindingCreateWithURL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 11 files


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

[Build Bot]

Comment on attachment 247376 [details]
Patch

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

New failing tests:
fast/attachment/attachment-rendering.html

[Build Bot]

Created attachment 247383 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

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

[Build Bot]

Comment on attachment 247376 [details]
Patch

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

New failing tests:
fast/attachment/attachment-rendering.html

[Build Bot]

Created attachment 247388 [details]
Archive of layout-test-results from ews101 for mac-mavericks

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

[Tim Horton]

Created attachment 247397 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  The parameter name "allocator" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  The parameter name "URL" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  _LSBindingCreateWithURL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 22 files


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

[Enrica Casucci]

Comment on attachment 247397 [details]
Patch

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

> Source/WebCore/css/CSSParser.cpp:811
>              return true;

I think you need #if ENABLE(ATTACHMENT_ELEMENT)

> Source/WebCore/rendering/RenderThemeMac.h:174
>  private:

You probably need #if ENABLE(ATTACHMENT_ELEMENT)

[Enrica Casucci]

Comment on attachment 247397 [details]
Patch

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

> Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34
> +LSBindingRef _LSBindingCreateWithURL(CFAllocatorRef allocator, CFURLRef URL);

No need for parameter names.

[Tim Horton]

Comment on attachment 247397 [details]
Patch

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

>> Source/WebCore/css/CSSParser.cpp:811
>>              return true;
> 
> I think you need #if ENABLE(ATTACHMENT_ELEMENT)

Annoying. I'll re-order the values so that this works.

[Tim Horton]

Created attachment 247446 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/mac/LaunchServicesSPI.h:34:  _LSBindingCreateWithURL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 21 files


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

[Enrica Casucci]

Comment on attachment 247446 [details]
Patch

I don't feel qualified to review the RenderThemeMac.mm changes, The rest looks good to me.

[Dean Jackson]

Comment on attachment 247446 [details]
Patch

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

> Source/WebCore/rendering/RenderAttachment.cpp:68
> +bool RenderAttachment::isFocused() const
> +{
> +    return frame().selection().isFocusedAndActive() && document().focusedElement() == &attachmentElement();
> +}
> +
> +bool RenderAttachment::isSelectedOrFocused() const
> +{
> +    return isSelected() || isFocused();
> +}

Could these go in the header?

> Source/WebCore/rendering/RenderTheme.cpp:989
> +#if ENABLE(ATTACHMENT_ELEMENT)
> +void RenderTheme::adjustAttachmentStyle(StyleResolver&, RenderStyle&, Element*) const
> +{
> +}

What's the point of this?

[Sam Weinig]

Comment on attachment 247446 [details]
Patch

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

> Source/WebCore/html/HTMLAttachmentElement.cpp:83
> +const String HTMLAttachmentElement::filePath() const
> +{
> +    return m_file ? m_file->path() : String();
> +}
> +
> +const String HTMLAttachmentElement::fileName() const
> +{
> +    return m_file ? m_file->name() : String();
>  }

I don't think these helpers are too helpful.  Also, if you do keep it, remove the const from the "const String".

[Tim Horton]

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