browser chrome test: browser_privatebrowsing_windowtitle.js TEST-UNEXPECTED-FAIL

RESOLVED FIXED in Firefox 3.7a1

Status

()

defect
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: dougt, Assigned: dao)

Tracking

({intermittent-failure})

Trunk
Firefox 3.7a1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(status1.9.2 .2-fixed, status1.9.1 .9-fixed)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

10 years ago
Error 0 (Unknown error: 0).TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js | The window title for about:privatebrowsing is correct (outside private browsing mode) - Got Minefield, expected Would you like to start Private Browsing?

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1251519039.1251520845.28344.gz&fulltext=1
(Reporter)

Updated

10 years ago
Whiteboard: [orange]

Updated

10 years ago
Blocks: 438871

Updated

10 years ago
Summary: mochitest: browser_privatebrowsing_windowtitle.js TEST-UNEXPECTED-FAIL → browser chrome test: browser_privatebrowsing_windowtitle.js TEST-UNEXPECTED-FAIL

Comment 2

10 years ago
Posted patch Patch (v1) (obsolete) — Splinter Review
I think this failure is very similar to bug 500702.  I took a similar approach in fixing it.

The delayedStartup replacement code is pretty straightforward.  The updateTitlebar replacement code used a timer, because in one case (about:privatebrowsing loaded outside of the PB mode), the page's onload handler changes the title, so updateTitlebar would be called twice for that page.
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #398874 - Flags: review?(mconnor)

Updated

10 years ago
status1.9.1: --- → ?
Flags: wanted-firefox3.6?
Attachment #398874 - Flags: review?(mconnor) → review+

Updated

10 years ago
Keywords: checkin-needed
the patch needs unbitrotting

btw, i did unbitrot it, but running it locally caused a fail:

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/../browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js | The window title for about:privatebrowsing is correct (outside private browsing mode) - Got Private Browsing - Minefield, expected Would you like to start Private Browsing? - Minefield

since i can't be completely sure i have unbitrotted correctly (but that looked quite trivial) could you provide an updated patch? i can eventually test it.
Keywords: checkin-needed
Posted patch patch v1.1 (obsolete) — Splinter Review
on the other side, giving some breath to the main thread, and increasing timeout to 300ms (especially this), solves the issue for me.

Being dependant on a timeout this could still be fragile though.
Attachment #404816 - Flags: review?(ehsan.akhgari)
notice it was working even with a 200ms timeout, but i set 300ms because i'm a pessimist.

Comment 8

10 years ago
Comment on attachment 404816 [details] [diff] [review]
patch v1.1

Does the same patch with a 200ms timeout fail?

Comment 9

10 years ago
Comment on attachment 404816 [details] [diff] [review]
patch v1.1

(In reply to comment #8)
> (From update of attachment 404816 [details] [diff] [review])
> Does the same patch with a 200ms timeout fail?

Sorry, didn't notice comment 6.  Let's try this patch.  r=me
Attachment #404816 - Flags: review?(ehsan.akhgari) → review+
http://hg.mozilla.org/mozilla-central/rev/fb7a133e977b
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
(Assignee)

Comment 11

10 years ago
Comment on attachment 404816 [details] [diff] [review]
patch v1.1

>+    let _updateTitlebar = gBrowser.updateTitlebar;
>+    gBrowser.updateTitlebar = function() {

You should use the DOMTitleChanged event instead.
(Assignee)

Comment 12

10 years ago
(In reply to comment #2)
> updateTitlebar replacement code used a timer, because in one case
> (about:privatebrowsing loaded outside of the PB mode), the page's onload
> handler changes the title, so updateTitlebar would be called twice for that
> page.

Couldn't you just flag that case and ignore the first DOMTitleChanged event?
(Assignee)

Comment 13

10 years ago
Or just always wait for for the load event.
(Assignee)

Updated

10 years ago
Attachment #404816 - Flags: review-
(Assignee)

Updated

10 years ago
Depends on: 521564
(Assignee)

Updated

10 years ago
Attachment #404816 - Flags: review-
(Assignee)

Comment 14

10 years ago
Comment on attachment 404816 [details] [diff] [review]
patch v1.1

filed bug 521564
Flags: in-testsuite+
(Assignee)

Comment 15

10 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1255304100.1255305930.22006.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 16

10 years ago
Posted patch patch v2Splinter Review
Closing the other bug again and moving the patch here...
Assignee: ehsan.akhgari → dao
Attachment #398874 - Attachment is obsolete: true
Attachment #404816 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #405828 - Flags: review?(ehsan.akhgari)
(Assignee)

Updated

10 years ago
No longer depends on: 521564
Duplicate of this bug: 521564
(Assignee)

Updated

10 years ago
Depends on: 521766
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5-Unittest/1255561066.1255562757.14013.gz
Linux mozilla-1.9.1 test everythingelse on 2009/10/14 15:57:46
OS: Mac OS X → All
Hardware: x86 → All

Comment 19

10 years ago
Comment on attachment 405828 [details] [diff] [review]
patch v2

r=me.
Attachment #405828 - Flags: review?(ehsan.akhgari) → review+
(Assignee)

Comment 20

10 years ago
http://hg.mozilla.org/mozilla-central/rev/8ed6fd9b927a
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
can this land on 1.9.1 and 1.9.2?

Comment 22

9 years ago
Comment on attachment 405828 [details] [diff] [review]
patch v2

this orange is affecting the 1.9.2 tree:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6/1268151207.1268152185.479.gz

can we please approve this patch for branches?
Attachment #405828 - Flags: approval1.9.2.2?
Attachment #405828 - Flags: approval1.9.1.9?

Comment 23

9 years ago
Posted patch 1.9.1 patchSplinter Review
So, on 1.9.1, we would need the fix to bug 506437 for this patch.  That fix got approval-minused because it was merely a polish bug, but now it's needed for this orange fix.  I would think that it's safe to take this patch.
Attachment #431400 - Flags: approval1.9.1.9?

Updated

9 years ago
Attachment #405828 - Flags: approval1.9.2.2?
Attachment #405828 - Flags: approval1.9.1.9?
Comment on attachment 431400 [details] [diff] [review]
1.9.1 patch

Approved for 1.9.1.9, a=dveditz
Attachment #431400 - Flags: approval1.9.1.9? → approval1.9.1.9+
Depends on: 506437
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
(Assignee)

Comment 29

9 years ago
(In reply to comment #28)
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1283768278.1283769737.8441.gz

bug 593626
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago9 years ago
Resolution: --- → FIXED
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.