Bug 27246 - Add HTMLMediaElement::supportSave() and a HitTestResult::absoluteMediaURL() functions
Summary: Add HTMLMediaElement::supportSave() and a HitTestResult::absoluteMediaURL() f...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-13 23:33 PDT by Albert J. Wong
Modified: 2009-07-15 08:21 PDT (History)
3 users (show)

See Also:


Attachments
Patch to add in HTMLMediaElement::supportsSave() and HitTestResult::absoluteMediaURL() (6.55 KB, patch)
2009-07-13 23:50 PDT, Albert J. Wong
no flags Details | Formatted Diff | Diff
Patch to add in HTMLMediaElement::supportsSave() and HitTestResult::absoluteMediaURL() (6.47 KB, patch)
2009-07-14 00:18 PDT, Albert J. Wong
darin: review+
Details | Formatted Diff | Diff
Undo the added delegation of HTMLMediaElement::supportsFullscreen(). (2.01 KB, patch)
2009-07-14 19:45 PDT, Albert J. Wong
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Albert J. Wong 2009-07-13 23:33:29 PDT
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.
Comment 1 Albert J. Wong 2009-07-13 23:50:47 PDT
Created attachment 32697 [details]
Patch to add in HTMLMediaElement::supportsSave() and HitTestResult::absoluteMediaURL()

See description.
Comment 2 Albert J. Wong 2009-07-14 00:18:53 PDT
Created attachment 32699 [details]
Patch to add in HTMLMediaElement::supportsSave() and HitTestResult::absoluteMediaURL()

removed some useless includes + forward declares from the last patch.
Comment 3 Brent Fulgham 2009-07-14 15:21:10 PDT
Landed in http://trac.webkit.org/changeset/45875.
Comment 4 Brent Fulgham 2009-07-14 16:04:02 PDT
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.
Comment 5 Brent Fulgham 2009-07-14 16:04:58 PDT
Corrected and applied in https://trac.webkit.org/changeset/45879.
Comment 6 Eric Carlson 2009-07-14 16:15:07 PDT
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.
Comment 7 Albert J. Wong 2009-07-14 19:45:55 PDT
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. :(
Comment 8 Holger Freyther 2009-07-15 08:21:49 PDT
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.