<rdar://33496301>
Created attachment 322043 [details] Patch
Comment on attachment 322043 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322043&action=review > Source/WebKit/ChangeLog:5 > + <rdar://problem/33496301> This should be 5692679; the one you mentioned is a dupe. > Source/WebKit/WebProcess/Plugins/PDF/PDFLayerControllerSPI.h:71 > +@property (nonatomic, strong) NSString* URLFragment; The asterisk is on the wrong side; we put them on the left for C++ types and on the right for ObjC types (it's weird, I know). > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1039 > + m_pdfLayerController.get().URLFragment = pdfURL.substring(index); This should get wrapped in an __MAC_OS_X_VERSION_MIN_REQUIRED ifdef, because it's not available in shipping versions of macOS.
Comment on attachment 322043 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322043&action=review > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1037 > + size_t index = pdfURL.reverseFind('#'); I bet our URL parser has a better way to do this! We should ask Alex. >> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1039 >> + m_pdfLayerController.get().URLFragment = pdfURL.substring(index); > > This should get wrapped in an __MAC_OS_X_VERSION_MIN_REQUIRED ifdef, because it's not available in shipping versions of macOS. Also I generally prefer the [m_pdfLayerController setURLFragment:] variant instead of dot notation in the case where you've got a RetainPtr. (.get() is so ugly)
Comment on attachment 322043 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322043&action=review >> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1037 >> + size_t index = pdfURL.reverseFind('#'); > > I bet our URL parser has a better way to do this! We should ask Alex. Actually, I don't even think we need to ask Alex; URL has fragmentIdentifier()
(In reply to Tim Horton from comment #2) > Comment on attachment 322043 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=322043&action=review > > > Source/WebKit/ChangeLog:5 > > + <rdar://problem/33496301> > > This should be 5692679; the one you mentioned is a dupe. > > > Source/WebKit/WebProcess/Plugins/PDF/PDFLayerControllerSPI.h:71 > > +@property (nonatomic, strong) NSString* URLFragment; > > The asterisk is on the wrong side; we put them on the left for C++ types and > on the right for ObjC types (it's weird, I know). > > > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1039 > > + m_pdfLayerController.get().URLFragment = pdfURL.substring(index); > > This should get wrapped in an __MAC_OS_X_VERSION_MIN_REQUIRED ifdef, because > it's not available in shipping versions of macOS. If I understand correctly, this should be a MIN_REQUIRED check in the implementation file and a MAX_ALLOWED check in PDFLayerControllerSPI.h?
Created attachment 322096 [details] Patch
(In reply to Aishwarya Nirmal from comment #0) > <rdar://33496301> The original bug is <rdar://5692679>
The title from 5692679 would have been preferable (it makes more sense), for future reference.
Comment on attachment 322096 [details] Patch Clearing flags on attachment: 322096 Committed r222620: <http://trac.webkit.org/changeset/222620>
All reviewed patches have been landed. Closing bug.
Minor follow up in https://trac.webkit.org/changeset/222624/webkit
This feature is working in Safari for MacOS but still isn't working for Safari on iOS/iPadOS. Could you please fix this? https://helpx.adobe.com/acrobat/kb/link-html-pdf-page-acrobat.html