Bug 143042

Summary:

iOS file upload panel menu items need icons

Product:

WebKit

Reporter:

Jon Honeycutt <jhoneycutt>

Component:

Forms

Assignee:

Jon Honeycutt <jhoneycutt>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aestes, ddkilzer, joepeck

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	aestes: review+

Description Jon Honeycutt 2015-03-25 01:03:52 PDT

The new iOS file upload panel menu items need icons for "Photo Library" and "Take a Photo or Video."

<rdar://problem/20178678>

Comment 1 Jon Honeycutt 2015-03-25 01:07:38 PDT

Created attachment 249390 [details]
Patch

Comment 2 Jon Honeycutt 2015-03-26 16:27:00 PDT

Created attachment 249538 [details]
Patch

Comment 3 Andy Estes 2015-03-26 17:06:43 PDT

Comment on attachment 249538 [details]
Patch

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

r=me

> Source/WebKit2/ChangeLog:11
> +        Declare new SPI methods +_webkit_takePhotoOrVideoIcon and
> +        +_webkit_photoLibraryIcon on UIDocumentMenuViewController.

_UIImageGetWebKitTakePhotoOrVideoIcon and _UIImageGetWebKitPhotoLibraryIcon().

> Source/WebKit2/Platform/spi/ios/UIKitSPI.h:40
> +#import <UIKit/UIInterface_Private.h>

You should go ahead and add function declarations to the !USE(APPLE_INTERNAL_SDK) block. Otherwise, there'll be a compiler error lurking that we won't catch until someone tries to build against the iOS 9 public SDK.

> Source/WebKit2/UIProcess/ios/forms/WKFileUploadPanel.mm:62
> +static UIImage *photoLibraryIcon()

static inline?

> Source/WebKit2/UIProcess/ios/forms/WKFileUploadPanel.mm:72
> +static UIImage *cameraIcon()

static inline?

Comment 4 Andy Estes 2015-03-26 17:32:14 PDT

Comment on attachment 249538 [details]
Patch

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

>> Source/WebKit2/Platform/spi/ios/UIKitSPI.h:40
>> +#import <UIKit/UIInterface_Private.h>
> 
> You should go ahead and add function declarations to the !USE(APPLE_INTERNAL_SDK) block. Otherwise, there'll be a compiler error lurking that we won't catch until someone tries to build against the iOS 9 public SDK.

Actually, I forgot an important detail about C functions. Instead of declaring them only in the !USE(APPLE_INTERNAL_SDK) condition, you should declare them unconditionally. This allows us to catch cases where UIKit changes a function declaration in a source-compatible but binary-incompatible way (thanks to the One Definition Rule). In fact, we already do that for other functions in this file (see _UIApplicationLoadWebKit()).

Comment 5 Jon Honeycutt 2015-03-26 19:25:39 PDT

Committed r182049: <http://trac.webkit.org/changeset/182049>