Closed Bug 513420 Opened 15 years ago Closed 14 years ago

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

Categories

(Firefox :: Private Browsing, defect)

Product:
Firefox
Component:
Private Browsing
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3.7a1
Tracking Flags:
Tracking Status
status1.9.2 --- .2-fixed
status1.9.1 --- .9-fixed

People

(Reporter: dougt, Assigned: dao)

References

(
URL
)

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 2 obsolete files)

patch v2
2.33 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review
1.9.1 patch
8.17 KB, patch
dveditz
: approval1.9.1.9+
Details | Diff | Splinter Review 
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
Whiteboard: [orange]
Blocks: 438871
Summary: mochitest: browser_privatebrowsing_windowtitle.js TEST-UNEXPECTED-FAIL → browser chrome test: browser_privatebrowsing_windowtitle.js TEST-UNEXPECTED-FAIL
Attached 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)
status1.9.1: --- → ?
Flags: wanted-firefox3.6?
Attachment #398874 - Flags: review?(mconnor) → review+
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
Attached 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 on attachment 404816 [details] [diff] [review]
patch v1.1

Does the same patch with a 200ms timeout fail?
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
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Comment on attachment 404816 [details] [diff] [review]
patch v1.1

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

You should use the DOMTitleChanged event instead.
(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?
Or just always wait for for the load event.
Attachment #404816 - Flags: review-
Depends on: 521564
Attachment #404816 - Flags: review-
Comment on attachment 404816 [details] [diff] [review]
patch v1.1

filed bug 521564
Flags: in-testsuite+
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1255304100.1255305930.22006.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patch v2 — — Splinter 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)
No longer depends on: 521564
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 on attachment 405828 [details] [diff] [review]
patch v2

r=me.
Attachment #405828 - Flags: review?(ehsan.akhgari) → review+
http://hg.mozilla.org/mozilla-central/rev/8ed6fd9b927a
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
can this land on 1.9.1 and 1.9.2?
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?
Attached patch 1.9.1 patch — — Splinter 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?
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
(In reply to comment #28)
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1283768278.1283769737.8441.gz

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

Attachment

General

Creator:
Created:
Updated:
Size: