If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Port Bug 496832 - Disable media entries in context menu when media source is invalid

RESOLVED FIXED in seamonkey2.0b2

Status

SeaMonkey
General
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.0b2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 391177 [details] [diff] [review]
proposed patch

Straight patch, only excluding context-viewvideo which we don't have. See bug 496832 comment 5 for the reasoning behind the .src change.
Attachment #391177 - Flags: superreview?(neil)
Attachment #391177 - Flags: review?(neil)

Comment 1

8 years ago
Comment on attachment 391177 [details] [diff] [review]
proposed patch

>             if ( this.target instanceof Components.interfaces.nsIImageLoadingContent && this.target.currentURI  ) {
>                 this.onImage = true;
>                 var request = this.target.getRequest( Components.interfaces.nsIImageLoadingContent.CURRENT_REQUEST );
>                 if (request && (request.imageStatus & request.STATUS_SIZE_AVAILABLE))
>                     this.onLoadedImage = true;
>-                this.mediaURL = this.target.currentURI.spec;
>+                this.mediaURL = this.target.currentURI.spec || this.target.src;
If you look above you'll see that we don't consider broken images, therefore we don't need to fall back to the src property.

I'm undecided as to whether we should show video items for broken videos yet.
(Assignee)

Comment 2

8 years ago
Created attachment 391376 [details] [diff] [review]
patch v2
Attachment #391177 - Attachment is obsolete: true
Attachment #391376 - Flags: review?(neil)
Attachment #391177 - Flags: superreview?(neil)
Attachment #391177 - Flags: review?(neil)

Comment 3

8 years ago
Comment on attachment 391376 [details] [diff] [review]
patch v2

>+        this.setItemAttr( "context-savevideo", "disabled", !this.mediaURL );
Might want to do if (this.onVideo) for this etc.

>+          var hasError = ( this.target.error != null );
Don't need the ()s.
Attachment #391376 - Flags: review?(neil) → review+
(Assignee)

Comment 4

8 years ago
Created attachment 391624 [details] [diff] [review]
patch v3
[Checkin: Comment 5]

This is just to make sure I understood you correctly.
Attachment #391376 - Attachment is obsolete: true
Attachment #391624 - Flags: review?(neil)

Updated

8 years ago
Attachment #391624 - Flags: review?(neil) → review+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Comment on attachment 391624 [details] [diff] [review]
patch v3
[Checkin: Comment 5]


http://hg.mozilla.org/comm-central/rev/5f194ee88991
Attachment #391624 - Attachment description: patch v3 → patch v3 [Checkin: Comment 5]
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b2
You need to log in before you can comment on or make changes to this bug.