1406051 - Double context menu separator on image link when selecting text

Status: VERIFIED FIXED

(Firefox :: Menus, defect, P4)

Description

[kanata]

Attachment: double_separator.jpg

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171005100211

Steps to reproduce:

1. Select text.
2. Open the context menu by clicking on image link (e.g. on <a href=""><img src=""></a>).

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Fairly minor regression, doesn't affect functionality, marking wontfix for 56.

Comment 2

[Edouard Oger [:eoger]]

Attachment: Bug 1406051 - Don't show two separators in context menu.

Review commit: https://reviewboard.mozilla.org/r/188150/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/188150/

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8917142 [details]
Bug 1406051 - Don't show two separators in context menu.

https://reviewboard.mozilla.org/r/188150/#review194176

We should have a test for this. You should be able to add to browser/base/content/test/general/browser_contextmenu.js

::: browser/base/content/browser-sync.js:469
(Diff revision 1)
>                                contextMenu.onImage || contextMenu.onCanvas ||
>                                contextMenu.onVideo || contextMenu.onAudio ||
>                                contextMenu.onLink || contextMenu.onTextInput);
>  
> -    ["context-sendpagetodevice", "context-sep-sendpagetodevice"]
> +    // Avoids double separator on images with links.
> +    const hideSeparator = contextMenu.isContentSelected && contextMenu.onLink && contextMenu.onImage;

Can you break this up into multiple lines?

Comment 4

[Edouard Oger [:eoger]]

Comment on attachment 8917142 [details]
Bug 1406051 - Don't show two separators in context menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/188150/diff/1-2/

Comment 5

[Edouard Oger [:eoger]]

I don't have a strong opinion if we should uplift this to 57, it's a fairly minor visual regression. What do you think Liz?

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4730ce3175f8
Don't show two separators in context menu. r=jaws

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4730ce3175f8

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Given where we are in the cycle, I think this should probably ride the 58 train, but feel free to set the 57 status back to affected and request Beta approval if you feel strongly otherwise.

Comment 9

[Md. Majedul islam]

I have reproduced this bug with Nightly 58.0a1 (2017-10-08) on Ubuntu 16.04, 64 bit!

The fix is now verified on Latest Nightly 58.0a1.

Build ID 	20171022100058
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0

Comment 10

[Cornel Ionce [:noni] [Hubs QA]]

Thanks for the verification. Updating the tracking flags and bug status.