994037 - Translation infobar can be placed in the wrong tab

Status: VERIFIED FIXED

(Firefox :: Translation, defect)

Description

[Bogdan Maris [:bogdan_maris], Desktop QA]

Attachment: Screencast showing the issue

Reproducible on try build http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-e487810349d2/ (BuildID: 20140407110118)

Steps to reproduce:
1. Start Firefox.
2. Go to Options/Preferences>Tabs
3. Uncheck 'Don`t load tabs until selected'
4. Open two tabs, in one load https://www.google.fr/ and the other any other website eg. https://www.google.com/
5. Keep the focus on google.com tab and close Firefox.
6. Click 'Close tabs' in the Confirm close pop-up
7. Reopen Firefox and Restore Previous Session.

Expected results: Session is restored exactly the way it was.

Actual results: The translation infobar from google.fr tab is now on google.com tab. Another translation infobar can be opened in google.fr tab but the infobar from google.com does not go away until you close it.

Notes:
1. This issue is present on Windows 7, Windows XP, Windows 8.1 on a Surface Pro 2, Ubuntu 12.04 32bit and Mac OS X 10.9.2

Comment 1

[Bogdan Maris [:bogdan_maris], Desktop QA]

Alternative steps to reproduce:
1. Open http://www.google.com/
2. Enter 'google french website' in the search box
3. Open www.google.fr/‎ in a new tab by clicking on the scroll wheel or right click and 'Open Link in New Tab'

Comment 2

[Florian Quèze [:florian]]

Bogdan, I wonder if this bug could have disappeared with the changes I made in bug 971048 to support e10s windows. Could you please tell me if you can still reproduce this bug with the try server build in bug 971048 comment 9?

Comment 3

[Bogdan Maris [:bogdan_maris], Desktop QA]

Unfortunately the issue still reproduces using the try build from bug 971048 comment 9 in both normal and e10s window.

Comment 4

[Bogdan Maris [:bogdan_maris], Desktop QA]

Forgot to say I tested on all platforms and got the same result.

Comment 5

[Florian Quèze [:florian]]

Thanks!

Comment 6

[:Felipe Gomes (needinfo for replies!)]

I see a similar problem with any site that takes a while to load. Not sure if it's the same root cause:

1 - open a slow loading website, e.g. http://www.uol.com.br
2 - open a new tab with a different and quick site that won't trigger the translation bar for you (i.e., in your locale language)
3 - Leave tab #2 selected. When site #1 finishes loading, its language will be reported as detected on tab #2

Comment 7

[Florian Quèze [:florian]]

Mass move of translation bugs to the new Translation component.

Comment 8

[Asaf Romano (gone)]

Attachment: patch

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 8424988 [details] [diff] [review]
patch

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

The code change looks good, but I would like this to have a test.

In browser_translation_infobar.js I think you can add something that opens a second tab, then call showTranslationUI for the first tab (you'll need an additional aBrowser optional parameter), and check that the infobar is in the first tab and not in the second (selected) tab.

Comment 10

[Asaf Romano (gone)]

To test this properly, we'd need to pass the browser to both showTranslationUI and checkURLBarIcon, which are called multiple times across the test. My original patch for bug 988275 refactored the test in a way that would help here, but right now I'm not sure if that test is going to land, and if so, if it won't change much. So I'm holding off on this test until the other bug is resolved.

I could also just land this patch, resolve this bug, and cover the test in the other bug.

Comment 11

[Asaf Romano (gone)]

Testing will be cover in bug 1015933.

Comment 12

[Asaf Romano (gone)]

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

Comment 13

[Asaf Romano (gone)]

By the way, the status whiteboard says [qa+], but as long as the translation component is not built, testing options are somewhat limited.

Comment 14

[Florian Quèze [:florian]]

(In reply to Mano from comment #13)
> By the way, the status whiteboard says [qa+], but as long as the translation
> component is not built, testing options are somewhat limited.

For previous bugs, either we pushed the patch to try with 'translation' added in browser/components/moz.build and gave the try build to QA, or QA created their own build (either local builds, or try server builds). While obviously not an ideal process, this has let QA (and more specifically Bogdan) uncover quite a few interesting bugs.

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Also, the try builds that I've sent to Bogdan this past week contained the patch by Mano here. So the latest try build that I sent should be enough to verify this bug.

Comment 16

[Bogdan Maris [:bogdan_maris], Desktop QA]

Using the build from Felipe I was able to verify that the issue is fixed. Tested on Windows 7 64bit, Ubuntu 13.04 64bit and Mac OS X 10.9.2.

Comment 17

[Asaf Romano (gone)]

Thanks!

(Technically, this should be marked fixed only once it hits mozilla-central, but that should happen in the next hour or so)

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6c6cea4ac070