Bug 449522 replaced .imageURL with .mediaURL and breaks any extension that used the old property. http://dafizilla.wordpress.com/2009/06/05/gcontextmenus-imageurl-property-refactored-on-firefox-3-5x/ A quick search on the AMO MXR turns up around 50 affected add-ons: http://mxr-test.konigsberg.mozilla.org/addons/search?string=.imageURL

We could quickly add a simple getter for imageURL that returns mediaURL if (onCanvas || onImage) (from https://bugzilla.mozilla.org/show_bug.cgi?id=449522#c30)

This was fixed for Firefox 3.1b2. So no extension author has taken care of it until now? There was a lot of time for. Is this really late-compat?

(In reply to comment #2) > This was fixed for Firefox 3.1b2. So no extension author has taken care of it > until now? There was a lot of time for. Is this really late-compat? When advising add-on devs about changes, we went by this URL: https://developer.mozilla.org/en/Firefox_3.5_for_developers I don't see this change listed there. Is it listed on that page under a different name perhaps?

Here is the other article that we used as a reference: https://developer.mozilla.org/En/Updating_extensions_for_Firefox_3.5

(In reply to comment #2) > This was fixed for Firefox 3.1b2. So no extension author has taken care of it > until now? There was a lot of time for. Is this really late-compat? Fixing it is potentially late-compat

If Mark's suggestion of creating a simple getter will resolve the issue, then I'd like to suggest we do that as it will: 1) Immediately resolve the issu 2) Not give add-on developers the impression that we're dropping something on them at the last minute

I'd take a patch that wallpapers over this, but this doesn't block. As Henrik said in comment #2, this is something that's existed since b2 which was before we declared "no more changes for add-on compatibility". Yes it will hurt some add-ons, but we've been on the up-and-up, here. (removing late-compat keyword, as it's being misapplied here)

(In reply to comment #7) > (removing late-compat keyword, as it's being misapplied here) I disagree here. The fix for this bug involves adding a property to an API that extensions use. That property might have been there 6 months ago but for the past 3 betas and at the point when we claimed there would be no more changes that impacted extensions it did not exist.

Adds a simple getter for imageURL

Comment on attachment 382301 [details] [diff] [review] patch I don't think arguing about whether late-compat applies is useful - we did say we'd potentially change things before b4, but we still have a fairly high bar for extension-compat causing changes no matter what point we're at in the release cycle, and I probably have pushed back on this change harder to begin with (supporting the old property is pretty low-cost). I also think that's it's highly unlikely that adding it back (even at this stage) is going to cause trouble. I think a better criterion for "late-compat" should really just be "anything we want extension developers/documentation writers/evangelists to look at near the end of a release cycle", and I think this bug satisfies it. >diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js >+ get imageURL { >+ if (this.onCanvas || this.onImage) I think the onCanvas check here is unnecessary - we didn't used to set imageURL for canvas elements, and it looks like we still don't set mediaURL in that case, so this can just check onImage. r=me with that change.

(In reply to comment #7) > I'd take a patch that wallpapers over this, but this doesn't block. As Henrik > said in comment #2, this is something that's existed since b2 which was before > we declared "no more changes for add-on compatibility". Yes it will hurt some > add-ons, but we've been on the up-and-up, here. > > (removing late-compat keyword, as it's being misapplied here) Mike, I have to disagree here. We (the AMO team) were told that changes would be documented so we could get the word out. This doesn't seem to have happened as far as I can tell which is why I linked to the documents which we used as a reference. Mark has already submitted a patch which would essentially negate a headache for 50 add-on developers for a modification that wasn't properly documented on our side. I think it's only fair to our community to fix this.

Removed the onCanvas check Added parens to the getter

(In reply to comment #11) > I think it's only fair to our community to fix this. Mike already said he'd take a patch, and Mark has one. So we are fixing this :) I think everyone's in agreement on what the best way forward is here, so there's no point haggling over the bug's keyword annotations.

(In reply to comment #13) > (In reply to comment #11) > > I think it's only fair to our community to fix this. > > Mike already said he'd take a patch, and Mark has one. So we are fixing this :) > > I think everyone's in agreement on what the best way forward is here, so > there's no point haggling over the bug's keyword annotations. Thanks Gavin. This is great news. I gotta advocate for the community. ;)

This change was never documented because nobody told anyone that works on documentation about it. I'm updating the docs to mention the change and the patch both.

Landed this on mozilla-central: https://hg.mozilla.org/mozilla-central/rev/5bb2498771c4

See https://developer.mozilla.org/En/Updating_extensions_for_Firefox_3.5#Changes_to_context_menus

(In reply to comment #17) > See > https://developer.mozilla.org/En/Updating_extensions_for_Firefox_3.5#Changes_to_context_menus Looks good, with one minor nit: the imageURL getter we're re-adding behaves the same way as the old imageURL property (it's only set if the context menu target is an image), so it's not quite an alias to mediaURL (which is also set for video/audio elements).

Verified fixed on trunk and 1.9.1 branch: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090711 Minefield/3.6a1pre ID:20090711031617 Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 ID:20090624012136