Bug 79383 – Remove obsolete File attributes

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alexey Proskuryakov]

// FIXME: obsolete attributes. To be removed.
        readonly attribute DOMString fileName;
        readonly attribute unsigned long long fileSize;

[Alexey Proskuryakov]

Created attachment 128531 [details]
proposed patch

Talked to Tim Hatcher on IRC, he's fine with removing these from public interfaces.

[Alexey Proskuryakov]

Created attachment 128537 [details]
with build fix

[Jian Li]

This is a breaking change. Alex, do you know how many web pages might be still accessing fileName and fileSize?

Fisher, is it fine to stop supporting these obsolete attributes in Chromium?

[Alexey Proskuryakov]

I do not know how much content would break, but if we ever plan to drop these attributes, now is a great time, in my opinion.

[WebKit Review Bot]

Comment on attachment 128537 [details]
with build fix

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

New failing tests:
http/tests/security/clipboard/clipboard-file-access.html
editing/pasteboard/file-input-files-access.html

[Alexey Proskuryakov]

Created attachment 128573 [details]
with updated tests

[Darin Fisher (:fishd, Google)]

It seems like it would be helpful to announce deprecation / removal of APIs on webkit-dev, and give port maintainers a chance to review the change.

For Chromium, we might want to look to see if any Chrome Extensions are using these attributes before we remove them.  See for example the webkit-dev thread about removing HTML notifications from WebKit.

I'm also a little bit concerned about the possibility of fileName and fileSize being used on the web since Firefox also implements those attributes:
https://developer.mozilla.org/en/DOM/File

Alexey, have you spoken with any Firefox devs about removing these attributes?  What makes you confident that we can make this change without breaking web sites?

[Alexey Proskuryakov]

Just getting my feet wet by following a comment left by someone else, CC'ing people in the know and waiting for feedback. Thanks for the pointers!

[WebKit Commit Bot]

Please wait for approval from [e-mail] (or another member of the Apple Safari Team) before submitting because this patch contains changes to the Apple Mac WebKit.framework public API.

[Alexey Proskuryakov]

Firefox 10 doesn't support these - Jonas says that they were removed a few versions ago.

[Darin Fisher (:fishd, Google)]

I'm glad to hear that Firefox has removed them too.  On the Chromium side, we can certainly be aggressive about disabling these in our nightly / dev channel builds.  Hopefully, if it breaks any websites (or extensions), that'll be enough for people to notice and file bug reports.

Would it make sense to do a smaller patch up front that just hides the DOM properties?  Should we worry about our ability to revert this patch if we find a month from now that it is doom-and-gloom?

[Alexey Proskuryakov]

This seems trivial to revert if needed, I'd prefer removing to hiding if it's OK with you.

[Hajime Morrita]

Comment on attachment 128573 [details]
with updated tests

Looks like there has been no further objection over weeks.
I agree that it's easy to revert this patch because the API is basically just alias or new name.
r+ing.

[WebKit Review Bot]

Comment on attachment 128573 [details]
with updated tests

Rejecting attachment 128573 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
youtTests/http/tests/security/clipboard/script-tests/clipboard-file-access.js is not empty after patch, as expected
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/http/tests/security/clipboard/script-tests/clipboard-file-access.js.rej
rm 'LayoutTests/http/tests/security/clipboard/script-tests/clipboard-file-access.js'

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Hajime Mor..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12035183

[Alexey Proskuryakov]

Created attachment 132960 [details]
updated for test changes

[WebKit Review Bot]

Comment on attachment 132960 [details]
updated for test changes

Rejecting attachment 132960 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Hajime Morrita found in /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog does not appear to be a valid reviewer according to committers.py.
ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12071120

[Alexey Proskuryakov]

Created attachment 132962 [details]
spell reviewer name per committers.py, not per Bugzilla name

[Hajime Morrita]

(In reply to comment #17)
> Created an attachment (id=132962) [details]
> spell reviewer name per committers.py, not per Bugzilla name
Oops. I'll align this once the patch landed.

[WebKit Review Bot]

Comment on attachment 132962 [details]
spell reviewer name per committers.py, not per Bugzilla name

Rejecting attachment 132962 [details] from commit-queue.

New failing tests:
http/tests/security/clipboard/clipboard-file-access.html
Full output: http://queues.webkit.org/results/12035269

[Alexey Proskuryakov]

Landed manually in <http://trac.webkit.org/changeset/111569>.