Bug 11388 - Stop using an iframe's id as fallback if its name attribute is not set
Summary: Stop using an iframe's id as fallback if its name attribute is not set
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
: 69236 185243 (view as bug list)
Depends on: 185322 186197
Blocks:
  Show dependency treegraph
 
Reported: 2006-10-23 03:50 PDT by Madhu M
Modified: 2018-07-23 18:31 PDT (History)
23 users (show)

See Also:


Attachments
Patch for review for the iframe name issue (2.62 KB, patch)
2006-10-25 05:02 PDT, Madhu M
mjs: review-
Details | Formatted Diff | Diff
Zip file containing samples to simulate the bug (1.36 KB, application/x-zip-compressed)
2006-10-25 05:13 PDT, Madhu M
no flags Details
Proposed patch for frame and iframe incorrect name assignment (11.59 KB, patch)
2011-10-05 04:28 PDT, Sameer Patil
rniwa: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Updated patch (19.05 KB, patch)
2012-02-07 04:22 PST, Sameer Patil
no flags Details | Formatted Diff | Diff
Updated patch (19.04 KB, patch)
2012-02-13 04:40 PST, Sameer Patil
no flags Details | Formatted Diff | Diff
Updated patch (18.55 KB, patch)
2012-02-15 00:33 PST, Sameer Patil
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Updated patch (40.23 KB, patch)
2012-02-15 03:51 PST, Sameer Patil
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Updated patch (73.41 KB, patch)
2012-02-16 22:59 PST, Sameer Patil
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (22.23 KB, patch)
2018-05-03 12:29 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (24.59 KB, patch)
2018-05-03 13:15 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (27.47 KB, patch)
2018-05-03 14:04 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.19 MB, application/zip)
2018-05-03 15:36 PDT, Build Bot
no flags Details
Patch (28.07 KB, patch)
2018-05-03 15:39 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (28.14 KB, patch)
2018-05-07 12:14 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-sierra (2.39 MB, application/zip)
2018-05-07 13:09 PDT, Build Bot
no flags Details
Patch (29.35 KB, patch)
2018-05-07 13:13 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (2.34 MB, application/zip)
2018-05-07 13:59 PDT, Build Bot
no flags Details
Patch (30.10 KB, patch)
2018-05-07 14:03 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Madhu M 2006-10-23 03:50:52 PDT
For iframe if the name is not specified, id is assigned to name.If both are not specified, name is getting assigned with self.name::<!--framePath //<!--frame0-->-->. If name(if not id) is given as _blank, it is getting the value as "_blank".

In Firefox if name is undefined or null then it will be shown as empty string.It will not take id as name, if id alone is given.If name is _blank, it will not get changed to ""
Comment 1 Alexey Proskuryakov 2006-10-23 06:00:31 PDT
We will likely need separate bugs for each of these issues. Please note that at least some of them are already known, e.g. bug 6751.
Comment 2 Madhu M 2006-10-25 05:02:46 PDT
Created attachment 11207 [details]
Patch for review for the iframe name issue

I have tried to fix this issue and make the behaviour similar to Firefox. All these issues are related.I have commented the code which is generating the unique name if the name of the frame is null or undefined. It will take the name as empty string in this case. If the name is not given and id is present, then also the name will be taken as "" instead of id
Comment 3 Madhu M 2006-10-25 05:13:48 PDT
Created attachment 11208 [details]
Zip file containing samples to simulate the bug

The attached ZIP is having sample files to show the bug. It is behaving differently in Safari and Firefox. After making the code changes as in the patch attached, it gives the same behaviour in both the browsers.
Comment 4 Madhu M 2006-10-30 04:49:56 PST
Please review the fix
Comment 5 Madhu M 2006-10-30 04:55:51 PST
Please review this bug and give comments about the fix
Comment 6 Maciej Stachowiak 2006-10-31 04:08:16 PST
Comment on attachment 11207 [details]
Patch for review for the iframe name issue

Thanks for the patch! It needs a couple of problems fixed to be landable.

1) This patch appears to be formatted as an RTF file or something (lots of weird backslashes). We need a plaintext patch to be able to land it.

2) The patch should not leave in commented code - if some lines of code are wrong, you should just remove them. There's also no need to put comments explaining why certain things aren't done.

3) The patch should include test cases in the form of tests and expected results for the LayoutTests directory, since it changes behavior for web content.  The samples should be part of the patch, not just a separate zip file.

4) I am pretty sure the unique frame names are for a reason, specifically for the back/forward list to be able to find things properly when you do back/forward in nested framesets. Did you test this case?

5) The patch needs to include a ChangeLog entry.

Thanks for the submission, please address these comments and submit a new patch. r- for now since this needs revision.
Comment 7 Mark Rowe (bdash) 2007-01-07 02:55:57 PST
Madhu, it'd be great if you could address Maciej's comments and resubmit this patch!
Comment 8 Alexey Proskuryakov 2011-10-03 12:37:34 PDT
*** Bug 69236 has been marked as a duplicate of this bug. ***
Comment 9 Sameer Patil 2011-10-05 04:28:48 PDT
Created attachment 109772 [details]
Proposed patch for frame and iframe incorrect name assignment

This issue looks generic and applies to frames too. Uploading proposed patch which solve this issue for frame and iframe. Uniquechildname issue adressed by Madhu in previous patch is fixed, please check https://bugs.webkit.org/show_bug.cgi?id=6751.I have modified LayoutTest/fast/frames/frame-element-name.html to incorporate test case related this issue.
Comment 10 WebKit Review Bot 2011-10-05 05:58:17 PDT
Comment on attachment 109772 [details]
Proposed patch for frame and iframe incorrect name assignment

Attachment 109772 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9959171

New failing tests:
http/tests/multipart/multipart-wait-before-boundary.html
http/tests/security/cross-frame-access-document-direct.html
http/tests/security/cross-frame-access-parent-explicit-domain.html
http/tests/security/cross-frame-access-port-explicit-domain.html
fast/dom/Window/slow-unload-handler.html
http/tests/navigation/no-referrer-subframe.html
http/tests/loading/redirect-methods.html
fast/events/open-window-from-another-frame.html
fast/events/onbeforeunload-focused-iframe.html
fast/dom/wrapper-context.html
http/tests/navigation/forward-and-cancel.html
http/tests/security/cross-frame-access-protocol-explicit-domain.html
http/tests/security/cross-frame-access-port.html
fast/dom/xss-DENIED-javascript-variations.html
http/tests/security/cross-frame-access-protocol.html
fast/dom/Window/window-special-properties.html
http/tests/misc/location-replace-crossdomain.html
fast/dom/Window/slow-unload-handler-only-frame-is-stopped.html
http/tests/navigation/image-load-in-subframe-unload-handler.html
http/tests/security/cross-frame-access-child-explicit-domain.html
Comment 11 Adam Barth 2011-10-05 10:26:54 PDT
Does Firefox still differ from our behavior here?  Does the HTML5 spec have a preference between the two behaviors?  What about IE?
Comment 12 Boris Zbarsky 2011-10-05 10:32:23 PDT
(In reply to comment #11)
> Does Firefox still differ from our behavior here?

Yes.

> Does the HTML5 spec have a preference between the two behaviors?

Yes.  See bug 69236 which has all this information.
Comment 13 Sameer Patil 2011-10-05 22:48:01 PDT
(In reply to comment #11)
>What about IE?
IE9 matches the behavior with Firefox.
Comment 14 Ryosuke Niwa 2011-12-08 16:54:27 PST
Comment on attachment 109772 [details]
Proposed patch for frame and iframe incorrect name assignment

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

> Source/WebCore/ChangeLog:8
> +        Frame name getting assigned from id attribute if it's value is null.

Please refer to a relevant spec.

> Source/WebCore/ChangeLog:9
> +

Please mention that you've added a test case to fast/frames/frame-element-name.html
Comment 15 Sameer Patil 2012-02-07 04:22:41 PST
Created attachment 125812 [details]
Updated patch

Uploading updated patch, incorporating review comments.
Comment 16 Sameer Patil 2012-02-13 04:40:07 PST
Created attachment 126754 [details]
Updated patch

Re-uploading patch as previous patch is not applied properly.
Comment 17 Sameer Patil 2012-02-15 00:33:59 PST
Created attachment 127124 [details]
Updated patch
Comment 18 WebKit Review Bot 2012-02-15 01:18:40 PST
Comment on attachment 127124 [details]
Updated patch

Attachment 127124 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11521620

New failing tests:
http/tests/multipart/multipart-wait-before-boundary.html
http/tests/navigation/forward-and-cancel.html
http/tests/misc/location-replace-crossdomain.html
fast/autoresize/autoresize-with-iframe.html
http/tests/security/dataURL/xss-DENIED-from-data-url-to-data-url.html
http/tests/security/cross-frame-access-protocol.html
http/tests/loading/redirect-methods.html
http/tests/security/cross-frame-access-document-direct.html
http/tests/inspector/inspect-element.html
http/tests/security/dataURL/xss-DENIED-to-data-url-sub-frame-uppercase.html
http/tests/security/protocol-compare-case-insensitive.html
http/tests/security/dataURL/xss-DENIED-to-data-url-from-data-url.html
http/tests/navigation/no-referrer-subframe.html
http/tests/security/cross-frame-access-child-explicit-domain.html
http/tests/security/cross-frame-access-port-explicit-domain.html
http/tests/security/host-compare-case-insensitive.html
http/tests/security/dataURL/xss-DENIED-to-data-url-sub-frame.html
http/tests/security/dataURL/xss-DENIED-from-data-url-sub-frame-2-level.html
http/tests/navigation/image-load-in-subframe-unload-handler.html
http/tests/security/cross-frame-access-port.html
http/tests/security/dataURL/xss-DENIED-from-data-url-sub-frame.html
accessibility/aria-describedby-on-input.html
http/tests/security/cross-frame-access-protocol-explicit-domain.html
http/tests/security/dataURL/xss-DENIED-to-data-url-sub-frame-2-level.html
http/tests/security/dataURL/xss-DENIED-to-data-url-in-foreign-domain-subframe.html
http/tests/security/cross-frame-access-parent-explicit-domain.html
http/tests/security/dataURL/xss-DENIED-from-data-url-in-foreign-domain-subframe.html
fast/dom/Window/slow-unload-handler.html
fast/dom/Window/timeout-callback-scope.html
fast/dom/Window/slow-unload-handler-only-frame-is-stopped.html
Comment 19 WebKit Review Bot 2012-02-15 03:50:59 PST
Attachment 127124 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 756 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 20 Sameer Patil 2012-02-15 03:51:29 PST
Created attachment 127155 [details]
Updated patch

Uploading latest patch, fixing failing Layout tests.
Comment 21 WebKit Review Bot 2012-02-15 07:39:15 PST
Comment on attachment 127155 [details]
Updated patch

Attachment 127155 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11514857

New failing tests:
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttributeNS.html
http/tests/navigation/forward-and-cancel.html
http/tests/misc/location-replace-crossdomain.html
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-getAttribute-value.html
http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttributeNS.html
http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-getAttribute-value.html
http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-htmldom.html
http/tests/inspector/inspect-element.html
http/tests/security/javascriptURL/javascriptURL-execution-context-frame-location-htmldom.html
http/tests/security/xssAuditor/full-block-base-href.html
http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttributeNodeNS.html
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttributeNodeNS.html
http/tests/security/javascriptURL/xss-DENIED-from-javascript-url-in-foreign-domain-subframe.html
http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttributeNode.html
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttributeNode.html
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttribute.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame.html
http/tests/security/dataURL/xss-DENIED-from-data-url-to-data-url.html
accessibility/aria-describedby-on-input.html
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-htmldom.html
fast/dom/Window/window-collection-length-no-crash.html
http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttribute.html
http/tests/security/dataURL/xss-DENIED-from-data-url-in-foreign-domain-subframe.html
http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html
fast/dom/wrapper-context.html
fast/dom/Window/window-access-after-navigation.html
http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-sub-frame.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html
http/tests/security/javascriptURL/xss-DENIED-to-javascript-url-in-foreign-domain-subframe.html
http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-sub-frame-2-level.html
Comment 22 Sameer Patil 2012-02-16 22:59:52 PST
Created attachment 127529 [details]
Updated patch
Comment 23 WebKit Review Bot 2012-02-17 02:12:09 PST
Comment on attachment 127529 [details]
Updated patch

Attachment 127529 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11541374

New failing tests:
platform/chromium/plugins/get-url-with-iframe-target-no-crash.html
fast/events/popup-when-select-change.html
fast/events/popup-blocked-to-post-blank.html
fast/dom/wrapper-context.html
http/tests/navigation/forward-and-cancel.html
fast/dom/Window/window-access-after-navigation.html
http/tests/misc/location-replace-crossdomain.html
plugins/get-url-with-iframe-target.html
fast/dom/Window/window-special-properties.html
http/tests/security/dataURL/xss-DENIED-from-data-url-to-data-url.html
fast/overflow/scrollRevealButton.html
fullscreen/full-screen-frameset.html
fast/dom/Window/window-collection-length-no-crash.html
fast/history/saves-state-after-frame-nav.html
Comment 24 Chris Dumez 2018-05-03 10:37:07 PDT
*** Bug 185243 has been marked as a duplicate of this bug. ***
Comment 25 Chris Dumez 2018-05-03 12:29:32 PDT
Created attachment 339440 [details]
Patch
Comment 26 Chris Dumez 2018-05-03 13:15:36 PDT
Created attachment 339448 [details]
Patch
Comment 27 Chris Dumez 2018-05-03 14:04:11 PDT
Created attachment 339461 [details]
Patch
Comment 28 Geoffrey Garen 2018-05-03 14:12:13 PDT
Comment on attachment 339461 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:17
> +        This WebKit quirk was causing some Web-compatibility issue because it

issues
Comment 29 Build Bot 2018-05-03 15:36:45 PDT
Comment on attachment 339461 [details]
Patch

Attachment 339461 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7556037

New failing tests:
http/tests/quicklook/csp-header-ignored.html
Comment 30 Build Bot 2018-05-03 15:36:47 PDT
Created attachment 339477 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 31 Chris Dumez 2018-05-03 15:39:18 PDT
Created attachment 339478 [details]
Patch
Comment 32 WebKit Commit Bot 2018-05-03 16:18:35 PDT
Comment on attachment 339478 [details]
Patch

Clearing flags on attachment: 339478

Committed r231331: <https://trac.webkit.org/changeset/231331>
Comment 33 WebKit Commit Bot 2018-05-03 16:18:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 34 Radar WebKit Bug Importer 2018-05-03 16:19:23 PDT
<rdar://problem/39959117>
Comment 35 Chris Dumez 2018-05-04 11:44:54 PDT
Reverted r231331 for reason:

Caused a few tests to assert

Committed r231367: <https://trac.webkit.org/changeset/231367>
Comment 36 Chris Dumez 2018-05-07 12:14:50 PDT
Created attachment 339738 [details]
Patch
Comment 37 Build Bot 2018-05-07 13:09:22 PDT
Comment on attachment 339738 [details]
Patch

Attachment 339738 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7597465

New failing tests:
fast/dom/Geolocation/srcdoc-getCurrentPosition.html
http/tests/navigation/image-load-in-subframe-unload-handler.html
fast/dom/Geolocation/srcdoc-watchPosition.html
http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html
http/tests/security/contentSecurityPolicy/iframe-blank-url-programmatically-add-external-script.html
fast/xmlhttprequest/xmlhttprequest-no-file-access.html
Comment 38 Build Bot 2018-05-07 13:09:23 PDT
Created attachment 339743 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 39 Chris Dumez 2018-05-07 13:13:49 PDT
Created attachment 339744 [details]
Patch
Comment 40 Build Bot 2018-05-07 13:59:27 PDT
Comment on attachment 339744 [details]
Patch

Attachment 339744 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7598026

New failing tests:
http/tests/navigation/image-load-in-subframe-unload-handler.html
Comment 41 Build Bot 2018-05-07 13:59:29 PDT
Created attachment 339750 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 42 Chris Dumez 2018-05-07 14:03:16 PDT
Created attachment 339751 [details]
Patch
Comment 43 WebKit Commit Bot 2018-05-07 14:42:35 PDT
Comment on attachment 339751 [details]
Patch

Clearing flags on attachment: 339751

Committed r231456: <https://trac.webkit.org/changeset/231456>
Comment 44 WebKit Commit Bot 2018-05-07 14:42:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 45 mitz 2018-06-01 09:25:30 PDT
(In reply to WebKit Commit Bot from comment #43)
> Comment on attachment 339751 [details]
> Patch
> 
> Clearing flags on attachment: 339751
> 
> Committed r231456: <https://trac.webkit.org/changeset/231456>

This broke the macOS Colloquy app. See bug 186197.
Comment 46 Daniel Jalkut 2018-07-23 18:31:25 PDT
This changes is also breaking MarsEdit's rich text editor. Bug 187937.