177582 2017-09-27 16:50:31 -0700 Request for PDF URL targets for specific pages, sections, etc. 2022-05-12 08:46:44 -0700 1 1 1 Unclassified WebKit PDF WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 anirmal webkit-unassigned achristensen aestes commit-queue karim megan_gardner thorton wenson_hsieh oldest_to_newest 1353932 0 anirmal 2017-09-27 16:50:31 -0700 <rdar://33496301> 1353938 1 322043 anirmal 2017-09-27 16:57:56 -0700 Created attachment 322043 Patch 1353945 2 322043 thorton 2017-09-27 17:05:04 -0700 Comment on attachment 322043 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. 1353948 3 322043 thorton 2017-09-27 17:07:30 -0700 Comment on attachment 322043 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) 1353949 4 322043 thorton 2017-09-27 17:08:22 -0700 Comment on attachment 322043 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() 1354150 5 wenson_hsieh 2017-09-28 10:05:28 -0700 (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? 1354176 6 322096 anirmal 2017-09-28 10:32:06 -0700 Created attachment 322096 Patch 1354177 7 anirmal 2017-09-28 10:34:43 -0700 (In reply to Aishwarya Nirmal from comment #0) > <rdar://33496301> The original bug is <rdar://5692679> 1354205 8 thorton 2017-09-28 11:13:32 -0700 The title from 5692679 would have been preferable (it makes more sense), for future reference. 1354218 9 322096 commit-queue 2017-09-28 11:40:45 -0700 Comment on attachment 322096 Patch Clearing flags on attachment: 322096 Committed r222620: <http://trac.webkit.org/changeset/222620> 1354219 10 commit-queue 2017-09-28 11:40:46 -0700 All reviewed patches have been landed. Closing bug. 1354236 11 thorton 2017-09-28 12:35:37 -0700 Minor follow up in https://trac.webkit.org/changeset/222624/webkit 1868432 12 karim 2022-05-12 08:46:44 -0700 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 322043 2017-09-27 16:57:56 -0700 2017-09-28 10:32:03 -0700 Patch bug-177582-20170927165756.patch text/plain 2426 anirmal U3VidmVyc2lvbiBSZXZpc2lvbjogMjIyNTU1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDMxY2JiYTE0NTMxMGJhODY4 ZTY2OTg2YzgzYTQwOTg4YzdkZWZlOWQuLmJiZjFjMzIwYzhiMWJlZGFmNTg2NTAyNDhhNjc1NDEw YjNkZTllNTQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTctMDktMjcgIEFpc2h3YXJ5 YSBOaXJtYWwgIDxhbmlybWFsQGFwcGxlLmNvbT4KKworICAgICAgICBSZXF1ZXN0IGZvciBQREYg VVJMIHRhcmdldHMgZm9yIHNwZWNpZmljIHBhZ2VzLCBzZWN0aW9ucywgZXRjLgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc3NTgyCisgICAgICAgIDxy ZGFyOi8vcHJvYmxlbS8zMzQ5NjMwMT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKyAgICAgICAgCisgICAgICAgIFRoaXMgY2hhbmdlIHNldHMgdGhlIFVSTCBmcmFnbWVu dCBmb3Igc2Nyb2xsIGFuY2hvcmluZyBpbiBQREZzLgorCisgICAgICAgICogV2ViUHJvY2Vzcy9Q bHVnaW5zL1BERi9QREZMYXllckNvbnRyb2xsZXJTUEkuaDoKKyAgICAgICAgKiBXZWJQcm9jZXNz L1BsdWdpbnMvUERGL1BERlBsdWdpbi5tbToKKyAgICAgICAgKFdlYktpdDo6UERGUGx1Z2luOjpw ZGZEb2N1bWVudERpZExvYWQpOgorCiAyMDE3LTA5LTI2ICBBbGV4IENocmlzdGVuc2VuICA8YWNo cmlzdGVuc2VuQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUGFzcyBhIGNvbXBsZXRpb24gaGFuZGxl ciBpbnN0ZWFkIG9mIGEgV2ViRnJhbWVQb2xpY3lMaXN0ZW5lclByb3h5IGR1cmluZyBwb2xpY3kg ZGVjaXNpb25zCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvUGx1Z2lucy9Q REYvUERGTGF5ZXJDb250cm9sbGVyU1BJLmggYi9Tb3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvUGx1 Z2lucy9QREYvUERGTGF5ZXJDb250cm9sbGVyU1BJLmgKaW5kZXggYmQwYTZmMjU4M2Y2MzhlZjhk YmE1MzRkYmE0OTBmMTJkYjU3ZTM1Ni4uOThmN2QwMDlmMmY1YWM5OTlhMjlkZDUwMjE4M2RlMTBm MDFhNDEwNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1BsdWdpbnMvUERG L1BERkxheWVyQ29udHJvbGxlclNQSS5oCisrKyBiL1NvdXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9Q bHVnaW5zL1BERi9QREZMYXllckNvbnRyb2xsZXJTUEkuaApAQCAtNjgsNiArNjgsNyBAQCB0eXBl ZGVmIE5TX0VOVU0oTlNJbnRlZ2VyLCBQREZMYXllckNvbnRyb2xsZXJDdXJzb3JUeXBlKSB7CiBA cHJvcGVydHkgKHJldGFpbikgQ0FMYXllciAqcGFyZW50TGF5ZXI7CiBAcHJvcGVydHkgKHJldGFp bikgUERGRG9jdW1lbnQgKmRvY3VtZW50OwogQHByb3BlcnR5IChyZXRhaW4pIGlkPFBERkxheWVy Q29udHJvbGxlckRlbGVnYXRlPiBkZWxlZ2F0ZTsKK0Bwcm9wZXJ0eSAobm9uYXRvbWljLCBzdHJv bmcpIE5TU3RyaW5nKiBVUkxGcmFnbWVudDsKIAogLSAodm9pZClzZXRGcmFtZVNpemU6KENHU2l6 ZSlzaXplOwogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvUGx1Z2lucy9Q REYvUERGUGx1Z2luLm1tIGIvU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1BsdWdpbnMvUERGL1BE RlBsdWdpbi5tbQppbmRleCA4NmZlMzZiMWZmZDhhZDBjZWZlMjVhZDRkN2U4Y2E0NTE4YzZhODlh Li5lYWE3NjBiNzA1YzI0OWZjM2MzMGRjOGM5ZjY2ZjM5Y2M5N2Y2OWU0IDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvUGx1Z2lucy9QREYvUERGUGx1Z2luLm1tCisrKyBiL1Nv dXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9QbHVnaW5zL1BERi9QREZQbHVnaW4ubW0KQEAgLTEwMzIs NiArMTAzMiwxMSBAQCB2b2lkIFBERlBsdWdpbjo6cGRmRG9jdW1lbnREaWRMb2FkKCkKIAogICAg IGlmIChbZG9jdW1lbnQgaXNMb2NrZWRdKQogICAgICAgICBjcmVhdGVQYXNzd29yZEVudHJ5Rm9y bSgpOworCisgICAgU3RyaW5nIHBkZlVSTCA9IHdlYkZyYW1lKCktPnVybCgpLnN0cmluZygpOwor ICAgIHNpemVfdCBpbmRleCA9IHBkZlVSTC5yZXZlcnNlRmluZCgnIycpOworICAgIGlmIChpbmRl eCAhPSBub3RGb3VuZCkKKyAgICAgICAgbV9wZGZMYXllckNvbnRyb2xsZXIuZ2V0KCkuVVJMRnJh Z21lbnQgPSBwZGZVUkwuc3Vic3RyaW5nKGluZGV4KTsKIH0KIAogdm9pZCBQREZQbHVnaW46OnN0 cmVhbURpZFJlY2VpdmVSZXNwb25zZSh1aW50NjRfdCBzdHJlYW1JRCwgY29uc3QgVVJMJiwgdWlu dDMyX3QsIHVpbnQzMl90LCBjb25zdCBTdHJpbmcmIG1pbWVUeXBlLCBjb25zdCBTdHJpbmcmLCBj b25zdCBTdHJpbmcmIHN1Z2dlc3RlZEZpbGVuYW1lKQo= 322096 2017-09-28 10:32:06 -0700 2017-09-28 11:40:45 -0700 Patch bug-177582-20170928103205.patch text/plain 2539 anirmal U3VidmVyc2lvbiBSZXZpc2lvbjogMjIyNTU1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDMxY2JiYTE0NTMxMGJhODY4 ZTY2OTg2YzgzYTQwOTg4YzdkZWZlOWQuLjAyOTBmYmM5ZjcyYmI2NTg1NmNjYTVmYTM4NmE2MDU1 Y2Y2MzU2NmMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTctMDktMjggIEFpc2h3YXJ5 YSBOaXJtYWwgIDxhbmlybWFsQGFwcGxlLmNvbT4KKworICAgICAgICBSZXF1ZXN0IGZvciBQREYg VVJMIHRhcmdldHMgZm9yIHNwZWNpZmljIHBhZ2VzLCBzZWN0aW9ucywgZXRjLgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc3NTgyCisgICAgICAgIDxy ZGFyOi8vcHJvYmxlbS81NjkyNjc5PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorICAgICAgICAKKyAgICAgICAgVGhpcyBjaGFuZ2Ugc2V0cyB0aGUgVVJMIGZyYWdtZW50 IGZvciBzY3JvbGwgYW5jaG9yaW5nIGluIFBERnMuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1Bs dWdpbnMvUERGL1BERkxheWVyQ29udHJvbGxlclNQSS5oOgorICAgICAgICAqIFdlYlByb2Nlc3Mv UGx1Z2lucy9QREYvUERGUGx1Z2luLm1tOgorICAgICAgICAoV2ViS2l0OjpQREZQbHVnaW46OnBk ZkRvY3VtZW50RGlkTG9hZCk6CisKIDIwMTctMDktMjYgIEFsZXggQ2hyaXN0ZW5zZW4gIDxhY2hy aXN0ZW5zZW5Ad2Via2l0Lm9yZz4KIAogICAgICAgICBQYXNzIGEgY29tcGxldGlvbiBoYW5kbGVy IGluc3RlYWQgb2YgYSBXZWJGcmFtZVBvbGljeUxpc3RlbmVyUHJveHkgZHVyaW5nIHBvbGljeSBk ZWNpc2lvbnMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9QbHVnaW5zL1BE Ri9QREZMYXllckNvbnRyb2xsZXJTUEkuaCBiL1NvdXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9QbHVn aW5zL1BERi9QREZMYXllckNvbnRyb2xsZXJTUEkuaAppbmRleCBiZDBhNmYyNTgzZjYzOGVmOGRi YTUzNGRiYTQ5MGYxMmRiNTdlMzU2Li5iMjAwYTRmMTRiNGI1ZGVlNWJmZTM3NmVhYTc2Yjk0OTNm MGQyOTIxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvUGx1Z2lucy9QREYv UERGTGF5ZXJDb250cm9sbGVyU1BJLmgKKysrIGIvU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1Bs dWdpbnMvUERGL1BERkxheWVyQ29udHJvbGxlclNQSS5oCkBAIC02OCw2ICs2OCw5IEBAIHR5cGVk ZWYgTlNfRU5VTShOU0ludGVnZXIsIFBERkxheWVyQ29udHJvbGxlckN1cnNvclR5cGUpIHsKIEBw cm9wZXJ0eSAocmV0YWluKSBDQUxheWVyICpwYXJlbnRMYXllcjsKIEBwcm9wZXJ0eSAocmV0YWlu KSBQREZEb2N1bWVudCAqZG9jdW1lbnQ7CiBAcHJvcGVydHkgKHJldGFpbikgaWQ8UERGTGF5ZXJD b250cm9sbGVyRGVsZWdhdGU+IGRlbGVnYXRlOworI2lmIF9fTUFDX09TX1hfVkVSU0lPTl9NQVhf QUxMT1dFRCA+PSAxMDEzMDQKK0Bwcm9wZXJ0eSAobm9uYXRvbWljLCBzdHJvbmcpIE5TU3RyaW5n ICpVUkxGcmFnbWVudDsKKyNlbmRpZgogCiAtICh2b2lkKXNldEZyYW1lU2l6ZTooQ0dTaXplKXNp emU7CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9QbHVnaW5zL1BERi9Q REZQbHVnaW4ubW0gYi9Tb3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvUGx1Z2lucy9QREYvUERGUGx1 Z2luLm1tCmluZGV4IDg2ZmUzNmIxZmZkOGFkMGNlZmUyNWFkNGQ3ZThjYTQ1MThjNmE4OWEuLmFk MjBmMGYxMDc1YmU0YmQyNGE4YWY0M2NjMWRjYzk5YjViMWNkYjEgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJLaXQvV2ViUHJvY2Vzcy9QbHVnaW5zL1BERi9QREZQbHVnaW4ubW0KKysrIGIvU291cmNl L1dlYktpdC9XZWJQcm9jZXNzL1BsdWdpbnMvUERGL1BERlBsdWdpbi5tbQpAQCAtMTAzMiw4ICsx MDMyLDEzIEBAIHZvaWQgUERGUGx1Z2luOjpwZGZEb2N1bWVudERpZExvYWQoKQogCiAgICAgaWYg KFtkb2N1bWVudCBpc0xvY2tlZF0pCiAgICAgICAgIGNyZWF0ZVBhc3N3b3JkRW50cnlGb3JtKCk7 Ci19CiAKKyNpZiBfX01BQ19PU19YX1ZFUlNJT05fTUlOX1JFUVVJUkVEID49IDEwMTMwNAorICAg IFN0cmluZyBwZGZVUkxGcmFnbWVudCA9IHdlYkZyYW1lKCktPnVybCgpLmZyYWdtZW50SWRlbnRp ZmllcigpOworICAgIFttX3BkZkxheWVyQ29udHJvbGxlciBzZXRVUkxGcmFnbWVudDpwZGZVUkxG cmFnbWVudF07CisjZW5kaWYKK30KKyAgICAKIHZvaWQgUERGUGx1Z2luOjpzdHJlYW1EaWRSZWNl aXZlUmVzcG9uc2UodWludDY0X3Qgc3RyZWFtSUQsIGNvbnN0IFVSTCYsIHVpbnQzMl90LCB1aW50 MzJfdCwgY29uc3QgU3RyaW5nJiBtaW1lVHlwZSwgY29uc3QgU3RyaW5nJiwgY29uc3QgU3RyaW5n JiBzdWdnZXN0ZWRGaWxlbmFtZSkKIHsKICAgICBBU1NFUlRfVU5VU0VEKHN0cmVhbUlELCBzdHJl YW1JRCA9PSBwZGZEb2N1bWVudFJlcXVlc3RJRCk7Cg==