Closed Bug 994037 Opened 7 years ago Closed 7 years ago

Translation infobar can be placed in the wrong tab

Categories

(Firefox :: Translation, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 32

People

(Reporter: bogdan_maris, Assigned: mano)

References

Details

(Whiteboard: [translation] p=5 s=it-32c-31a-30b.3 [qa!])

Attachments

(2 files)

Reproducible on try build http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/florian@queze.net-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
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'
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?
Flags: needinfo?(bogdan.maris)
Unfortunately the issue still reproduces using the try build from bug 971048 comment 9 in both normal and e10s window.
Flags: needinfo?(bogdan.maris)
Forgot to say I tested on all platforms and got the same result.
Thanks!
Flags: firefox-backlog?
Flags: firefox-backlog? → firefox-backlog+
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
Mass move of translation bugs to the new Translation component.
Component: General → Translation
Version: 31 Branch → unspecified
Summary: Translation infobar is placed in the wrong tab with Don`t load tabs until selected unchecked → Translation infobar can be placed in the wrong tab
Whiteboard: [translation] → [translation] p=0
Whiteboard: [translation] p=0 → [translation] p=5
Assignee: nobody → mano
Status: NEW → ASSIGNED
Whiteboard: [translation] p=5 → [translation] p=5 s=it-32c-31a-30b.2 [qa?]
Whiteboard: [translation] p=5 s=it-32c-31a-30b.2 [qa?] → [translation] p=5 s=it-32c-31a-30b.2 [qa+]
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.
Attachment #8424988 - Flags: review?(florian) → review+
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.
By the way, the status whiteboard says [qa+], but as long as the translation component is not built, testing options are somewhat limited.
(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.
QA Contact: bogdan.maris
Whiteboard: [translation] p=5 s=it-32c-31a-30b.2 [qa+] → [translation] p=5 s=it-32c-31a-30b.3 [qa+]
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.
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.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [translation] p=5 s=it-32c-31a-30b.3 [qa+] → [translation] p=5 s=it-32c-31a-30b.3 [qa!]
Status: RESOLVED → VERIFIED
Thanks!

(Technically, this should be marked fixed only once it hits mozilla-central, but that should happen in the next hour or so)
Target Milestone: --- → Firefox 32
You need to log in before you can comment on or make changes to this bug.