Bug 158287 - Consolidate usage of NSAttachmentChar
Summary: Consolidate usage of NSAttachmentChar
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Safari 9
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-06-01 17:24 PDT by chris fleizach
Modified: 2016-06-16 20:25 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.47 KB, patch)
2016-06-01 17:47 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
Patch (3.48 KB, patch)
2016-06-01 17:50 PDT, chris fleizach
darin: review+
Details | Formatted Diff | Diff
BUG 158287 (93.64 KB, patch)
2016-06-12 17:04 PDT, Abu Naser Saifullah
no flags Details | Formatted Diff | Diff
http://www.bugs.webkit.org (5 bytes, text/plain)
2016-06-12 17:12 PDT, Abu Naser Saifullah
cfleizach: review-
cfleizach: commit-queue-
Details

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2016-06-01 17:24:01 PDT
There are 3 places where NSAttachmentCharacter lives in WebCore
Comment 1 Radar WebKit Bug Importer 2016-06-01 17:24:59 PDT
<rdar://problem/26591150>
Comment 2 chris fleizach 2016-06-01 17:47:13 PDT
Created attachment 280286 [details]
Patch
Comment 3 WebKit Commit Bot 2016-06-01 17:48:21 PDT
Attachment 280286 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:60:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 chris fleizach 2016-06-01 17:50:57 PDT
Created attachment 280287 [details]
Patch
Comment 5 WebKit Commit Bot 2016-06-01 17:53:03 PDT
Attachment 280287 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:60:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Darin Adler 2016-06-02 11:19:09 PDT
Comment on attachment 280287 [details]
Patch

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

> Source/WebCore/platform/mac/WebNSAttributedStringExtras.h:33
> +#if TARGET_OS_IOS
> +enum {
> +    NSAttachmentCharacter = 0xfffc    /* To denote attachments. */
> +};
> +#endif

It seems strange for us to define this inside the WebCore namespace.

This is defined in NSTextAttachment.h; can we use that instead? Is that not available on iOS?
Comment 7 chris fleizach 2016-06-02 11:19:44 PDT
(In reply to comment #6)
> Comment on attachment 280287 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=280287&action=review
> 
> > Source/WebCore/platform/mac/WebNSAttributedStringExtras.h:33
> > +#if TARGET_OS_IOS
> > +enum {
> > +    NSAttachmentCharacter = 0xfffc    /* To denote attachments. */
> > +};
> > +#endif
> 
> It seems strange for us to define this inside the WebCore namespace.
> 
> This is defined in NSTextAttachment.h; can we use that instead? Is that not
> available on iOS?

Correct, not available on iOS for some reason
Comment 8 Abu Naser Saifullah 2016-06-12 17:04:49 PDT
Created attachment 281133 [details]
BUG 158287
Comment 9 Abu Naser Saifullah 2016-06-12 17:12:02 PDT
Created attachment 281134 [details]
http://www.bugs.webkit.org
Comment 10 Abu Naser Saifullah 2016-06-12 17:15:04 PDT
Comment on attachment 281134 [details]
http://www.bugs.webkit.org

Edit Attachment
Comment 11 Abu Naser Saifullah 2016-06-12 17:16:20 PDT
Comment on attachment 281134 [details]
http://www.bugs.webkit.org

Edit attachment
Comment 12 Abu Naser Saifullah 2016-06-12 17:17:34 PDT
(In reply to comment #1)
> <rdar://problem/26591150>
Comment 13 chris fleizach 2016-06-12 22:41:00 PDT
Comment on attachment 281134 [details]
http://www.bugs.webkit.org

Spam?
Comment 14 Darin Adler 2016-06-16 20:25:21 PDT
Comment on attachment 280287 [details]
Patch

We should not define this in the WebCore namespace. We should just define it in the global namespace on iOS to match Mac, and then we can remove it when eventually it shows up in the iOS SDK as well.