Elbart Reporter
	Description • 10 years ago

Visit URL, Inspect any of the tiles, and check the "element" under "Rules".

Right-clicking on the URL of background-image shows the "Copy"-option disabled.

	thrnarrw
	Comment 1 • 10 years ago

39.0a1

Copy was disabled


http://i.imgur.com/rNxPaWP.png

	Tim Nguyen :ntim Assignee
	Comment 3 • 9 years ago

The test I've added didn't get caught in the last patch

	Tim Nguyen :ntim Assignee
	Comment 4 • 9 years ago

Fixed some nits I didn't catch in my previous patch

	Patrick Brosset <:pbro>
	Comment 5 • 9 years ago

Comment on attachment 8628868 [details] [diff] [review]
Patch v1.2

Review of attachment 8628868 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great! Thanks Tim.

::: browser/devtools/styleinspector/test/browser_styleinspector_context-menu-copy-urls.js
@@ +1,3 @@
>  /* Any copyright is dedicated to the Public Domain.
>     http://creativecommons.org/publicdomain/zero/1.0/ */
>  

Since you're changing this test, can you also take this opportunity to add "use strict"; ?

@@ +83,5 @@
> +  if (type == "data-uri") {
> +    info("Click Copy Data URI and wait for clipboard");
> +    yield waitForClipboard(() => view.menuitemCopyImageDataUrl.click(), expected);
> +  }
> +  else {

nit: Put else on the same line as the closing }

::: toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties
@@ +111,5 @@
>  styleinspector.contextmenu.copyImageDataUrl=Copy Image Data-URL
>  
>  # LOCALIZATION NOTE (styleinspector.contextmenu.copyDataUri.accessKey): Access key for
>  # the rule and computed view context menu "Copy Image Data-URL" entry.
> +styleinspector.contextmenu.copyImageDataUrl.accessKey1=I

Are access keys localized? Do we need a new key ID for this change?

	Tim Nguyen :ntim Assignee
	Comment 6 • 9 years ago

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #5)
> ::: toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties
> @@ +111,5 @@
> >  styleinspector.contextmenu.copyImageDataUrl=Copy Image Data-URL
> >  
> >  # LOCALIZATION NOTE (styleinspector.contextmenu.copyDataUri.accessKey): Access key for
> >  # the rule and computed view context menu "Copy Image Data-URL" entry.
> > +styleinspector.contextmenu.copyImageDataUrl.accessKey1=I
> 
> Are access keys localized? Do we need a new key ID for this change?

I changed the accesskey to accesskey1 in this patch so this will get caught by the l10n team

	Tim Nguyen :ntim Assignee
	Comment 7 • 9 years ago

Fixed nits.

	Tim Nguyen :ntim Assignee
	Comment 8 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ef44e608c9b

	Patrick Brosset <:pbro>
	Comment 9 • 9 years ago

(In reply to Tim Nguyen [:ntim] from comment #6)
> (In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #5)
> > ::: toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties
> > @@ +111,5 @@
> > >  styleinspector.contextmenu.copyImageDataUrl=Copy Image Data-URL
> > >  
> > >  # LOCALIZATION NOTE (styleinspector.contextmenu.copyDataUri.accessKey): Access key for
> > >  # the rule and computed view context menu "Copy Image Data-URL" entry.
> > > +styleinspector.contextmenu.copyImageDataUrl.accessKey1=I
> > 
> > Are access keys localized? Do we need a new key ID for this change?
> 
> I changed the accesskey to accesskey1 in this patch so this will get caught
> by the l10n team
Yeah but, do accesskeys need to be translated?
From https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices I see:

"
If you change the entity or property name of a string, and the string has an accompanying access key, command key, or tooltip, you should update their names as well for consistency. This change is fundamental for access keys, since many localization tools use the entity name to connect an access key to its label, for example to check if it's using a character not available in the original string: given entities "useBookmark.label" and "useBookmark.accesskey", if you change to "chooseBookmark.label" due to a string change, change the access key entity to "chooseBookmark.accesskey" to match it.
"

which makes me think that the word "accesskey" in the property name is important, and so we shouldn't change it.
Francesco? Any guidelines here?

	Francesco Lodolo [:flod]
	Comment 10 • 9 years ago

Comment on attachment 8629313 [details] [diff] [review]
Patch v1.3

Review of attachment 8629313 [details] [diff] [review]:
-----------------------------------------------------------------

To answer the questions.

1) Yes, accesskeys are localized. Unlike shortcuts (or commankeys), you need to match the label. 
"Show" with a fixed accesskey "w" wouldn't work in Italian, since "w" it's not available in the translation "Visualizza".

2) Yes, it's important to keep ".accesskey" to help tools figure out the association between label and accesskey. Not sure if they're smart enough to understand ".accesskey1", probably not.

3) Most important: you don't need to change the ID, simply update the accesskey for English. Given #1, each locale will have a completely different set of accesskeys, and they need to figure out on their own how to avoid conflicts.

	Tim Nguyen :ntim Assignee
	Comment 11 • 9 years ago

Restored the older accesskey name per previous comment

	Pulsebot
	Comment 12 • 9 years ago

https://hg.mozilla.org/integration/fx-team/rev/f7d5df6f37b7

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 13 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/f7d5df6f37b7

	Saheda Reza [:Antora]
	Comment 14 • 9 years ago

I found the bug on Firefox nightly windows 32bit-

Build ID 	20150206030205
User Agent 	Mozilla/5.0 (Windows NT 6.3; rv:38.0) Gecko/20100101 Firefox/38.0


And this looks fixed on :

Build ID 	20150708030204
User Agent 	Mozilla/5.0 (Windows NT 6.3; rv:42.0) Gecko/20100101 Firefox/42.0

	Ashickur Rahman
	Comment 15 • 9 years ago

Successfully reproduce the bug on Firefox 39 (Build ID 20150630154324) Linux 32 bit.

The fix works for me on Firefox 42.0a1, Build ID: 20150703030216, User Agent: Mozilla/5.0 (X11; Linux i686; rv:42.0) Gecko/20100101 Firefox/42.0 .

Based on the Comment 14 and my verification, fix is verified.