169826 - Port patch to bug 164006 (wyciwyg: in tab titles)

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P5)

Description

[Blake Ross]

Phoenix is missing the patch for bug 164006.  It touches tabbrowser and friends,
can you check it out and see if it's a good patch?  And bring it over if so?

Comment 1

[David Hyatt]

This patch adds a lot of code for uncommonly accessed URLs.  I see no reason 
to bloat Phoenix up with all this extra code to handle this.

Comment 2

[Brendan Eich [:brendan]]

This is not very important, but it seems like a bug worth fixing if Phoenix
already uses the URIFixup stuff -- a tab's default title can come from the same
fixed-up URL that goes in the location bar.  Why not be consistent?  Hyatt said
I could leave this untargeted on blake's list for now.

/be

Comment 3

[Brendan Eich [:brendan]]

For later.

/be

Comment 4

[Brendan Eich [:brendan]]

Also, this might be WORKSFORME already -- Asa, can you test?

/be

Comment 5

[Carl Schrader]

I checked the URL supplied in bug 164006 and the tab is Titled Untitled. This is
using the latest Phoenix nightly (10/30/02). 

Comment 6

[Asa Dotzler [:asa]]

I don't think this is fixed. Testing on a recent Linux build I don't see fixed
up page title  in the tab title.

Comment 7

[Michael Newton]

No, this isn't fixed on latest Windows builds.

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3a) Gecko/20021231 Phoenix/0.5

Comment 8

[Pham]

*** Bug 215627 has been marked as a duplicate of this bug. ***

Comment 9

[Patrick McCormick]

Almost all of the patch in bug 164006 has been added to Firebird some time ago, 
except for the lines in tabbrowser.xml which actually execute all the other 
stuff.

Patch coming up for the tabbrowser.xml segment only.

Comment 10

[Patrick McCormick]

Attachment: patch for tabbrowser.xml

Comment 11

[Jesse Ruderman]

*** Bug 241411 has been marked as a duplicate of this bug. ***

Comment 12

[Mike Connor [:mconnor]]

*** Bug 243358 has been marked as a duplicate of this bug. ***

Comment 13

[Patrick McCormick]

Comment on attachment 140843 [details] [diff] [review]
patch for tabbrowser.xml

bz, maybe you can take a look at this?	patch still applies to tip.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 140843 [details] [diff] [review]
patch for tabbrowser.xml

This needs review from firefox people.	That wouldn't be me.

Comment 15

[Jeff Walden [:Waldo]]

Comment on attachment 140843 [details] [diff] [review]
patch for tabbrowser.xml

Okay, let's try mconnor for the review, then.  If the patch is to get in, it
should probably do so before 1.0 as a polish bug.

Comment 16

[Blake Ross]

Load balancing my bugs...

Mike, since you're already flagged for review here, can you drive this one?

Comment 17

[Asa Dotzler [:asa]]

p4 priority - not a blocker. if a fully reviewed patch materializes, please
nominate for aviary approval. 

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Attachment: Updated patch - cleans obsolete entries from it

This is the same patch as before, but it removes a couple hunks that have
already found their way into the source tree.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

One problem I see with this patch. According to browser.js, "about:blank" in the
location bar should be changed to "".  With the testcase above, that doesn't
happen.  Any way to fix that?

Comment 20

[Ryan VanderMeulen [:RyanVM]]

This patch in its current form causes a regression. Bug 279074 was fixed when I
removed this patch.

Comment 21

[:Gavin Sharp [email: [e-mail]]]

Attachment: Updated Patch

Fixes the remaining issues. First hunk is explained at bug 164006 comment 15,
second hunk is using:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/browser.xml&mark=308#307


One tiny step closer to a merged tabbrowser! :)

Comment 22

[:Gavin Sharp [email: [e-mail]]]

Oh yeah, and most of this went in with bug 280438.

Comment 23

[:Gavin Sharp [email: [e-mail]]]

Attachment: Updated once again

All the functional stuff has been taken care of, this just synchronizes
tabbrowser.xml between toolkit and xpfe. (second part is just a tweak of the
change made for 281988, the first is explained at bug 164006 comment 15).

Comment 24

[Asaf Romano (gone)]

Checking in content/widgets/tabbrowser.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbrowser.xml,v  <--  tabbrowser.xml
new revision: 1.83; previous revision: 1.82
done

Comment 25

[Asaf Romano (gone)]

backed out, due to bug 297037.

Comment 26

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch v235

Last time, promise! :)

browser.js's onLocationChange needed to be tweaked to deal with the change.

Comment 27

[Bradley Chapman (not reading bugmail, still gone but not forever)]

I just went to bug 164006 and tested the URL in that field and was unable to
reproduce the original bug. Is this FIXED?

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b5) Gecko/20051013 Firefox/1.4.1
ID:2005101304

Comment 28

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #27)
> I just went to bug 164006 and tested the URL in that field and was unable to
> reproduce the original bug. Is this FIXED?

It's mostly fixed, because of patches in various other bugs. The only remaining changes are the tabbrowser.xml ones from the previous patch.

Comment 29

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Comment 30

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/base/content/browser.js 	1.721
mozilla/toolkit/content/widgets/tabbrowser.xml 	1.208

I tweaked the previous patch to not cause bug 297037, and carried over r=mconnor.

Comment 31

[Nickolay_Ponomarev]

Granted, I don't understand this patch, but is it really intended that extension-registered nsIWebProgressListeners can now get an onLocationChange notification with null nsIURI param? I don't know if it was possible before this patch, but seems rather counter-intuitive just from looking at nsIWebProgressListener docs...

If this is intentional, we should clarify the nsIWebProgressListener docs and include a note about this change somewhere in Fx3 documentation.