Bug 27246 – Add HTMLMediaElement::supportSave() and a HitTestResult::absoluteMediaURL() functions

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

[Albert J. Wong]

Currently, the HitTestResult does not store enough info to access a media's element source URL for saving and Copy Address.

Expose the media element's source url in the HitTestResult so that it may be used by the context menu for Copy Address, Open in new Tab/Window, and maybe Save As depending on how the port caches the media file.

Also add a stub supportSave() function into HTMLMediaElement and MediaPlayerPrivateInterface so that the Media Engine can control whether or not the save control will be enabled.

[Albert J. Wong]

Created attachment 32697 [details]
Patch to add in HTMLMediaElement::supportsSave() and HitTestResult::absoluteMediaURL()

See description.

[Albert J. Wong]

Created attachment 32699 [details]
Patch to add in HTMLMediaElement::supportsSave() and HitTestResult::absoluteMediaURL()

removed some useless includes + forward declares from the last patch.

[Brent Fulgham]

Landed in http://trac.webkit.org/changeset/45875.

[Brent Fulgham]

This patch caused crashes in various media tests.  When making a change you should run the corresponding tests (ideally, all tests) to ensure no problems.

The original patch had two big errors:

 851 
 852 bool HTMLMediaElement::supportsFullscreen() const
 853 {
 854     return m_player->supportsFullscreen();
 855 }
 856 
 857 bool HTMLMediaElement::supportsSave() const
 858 {
 859     return m_player->supportsSave();
 860 }

These two lines should be checking if m_player is null, as this will happen (frequently).

A quick run of:  'run-webkit-tests media' would have shown this error on the fourth test.

[Brent Fulgham]

Corrected and applied in https://trac.webkit.org/changeset/45879.

[Eric Carlson]

Also HTMLMediaElement::supportsFullscreen shouldn't have been changed unless you want to allow <audio> elements to go fullscreen, and if that is the goal HTMLVideoElement::supportsFullscreen should be removed.

[Albert J. Wong]

Created attachment 32760 [details]
Undo the added delegation of HTMLMediaElement::supportsFullscreen().

Per Eric's comment, undoing this part of my earlier patch.

I've verified that the media tests pass this time.

Sorry for the bustage in the last patch.  I thought that the version of the layout tests in the chromium build were equivalent in coverage.  Learned that I was completely wrong. :(

[Holger Freyther]

Comment on attachment 32760 [details]
Undo the added delegation of HTMLMediaElement::supportsFullscreen().

Sounds reasonable, I take that we don't want MediaControlElement to know about audio/video.