Closed Bug 169826 Opened 17 years ago Closed 13 years ago

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

Categories

(Firefox :: Tabbed Browser, defect, P5)

defect

Tracking

()

RESOLVED FIXED
Future

People

(Reporter: bugzilla, Assigned: Gavin)

References

()

Details

Attachments

(1 file, 5 obsolete files)

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?
Target Milestone: --- → Phoenix0.2
Target Milestone: Phoenix0.2 → Phoenix0.3
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.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → WONTFIX
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
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
For later.

/be
Assignee: hyatt → blaker
Status: REOPENED → NEW
Also, this might be WORKSFORME already -- Asa, can you test?

/be
Target Milestone: Phoenix0.3 → ---
I checked the URL supplied in bug 164006 and the tab is Titled Untitled. This is
using the latest Phoenix nightly (10/30/02). 
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.
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
Summary: Evaluate fix for 164006 and port it to Phoenix if necessary → Evaluate fix for 164006 and port it to Mozilla Firebird if necessary
Target Milestone: --- → After Firebird 1.0
*** Bug 215627 has been marked as a duplicate of this bug. ***
QA Contact: asa
Summary: Evaluate fix for 164006 and port it to Mozilla Firebird if necessary → Port patch to bug 164006 (wyciwyg: in tab titles)
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.
Attached patch patch for tabbrowser.xml (obsolete) — Splinter Review
Attachment #140843 - Flags: review+
Attachment #140843 - Flags: review+ → review?(bugs)
Attachment #140843 - Flags: superreview?(blake)
Attachment #140843 - Flags: review?(bugs)
Attachment #140843 - Flags: review?(blake)
Attachment #140843 - Flags: superreview?(firefox)
*** Bug 241411 has been marked as a duplicate of this bug. ***
*** Bug 243358 has been marked as a duplicate of this bug. ***
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.
Attachment #140843 - Flags: review?(firefox) → review?(bzbarsky)
Comment on attachment 140843 [details] [diff] [review]
patch for tabbrowser.xml

This needs review from firefox people.	That wouldn't be me.
Attachment #140843 - Flags: review?(bzbarsky)
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.
Attachment #140843 - Flags: review?(mconnor)
Flags: blocking1.0?
Flags: blocking1.0? → blocking1.0+
Load balancing my bugs...

Mike, since you're already flagged for review here, can you drive this one?
Assignee: firefox → mconnor
p4 priority - not a blocker. if a fully reviewed patch materializes, please
nominate for aviary approval. 
Flags: blocking-aviary1.0+ → blocking-aviary1.0-
Component: General → Tabbed Browser
OS: Windows XP → All
QA Contact: firefox.tabbed-browser
Hardware: PC → All
This is the same patch as before, but it removes a couple hunks that have
already found their way into the source tree.
Attachment #171456 - Flags: review?(mconnor)
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?
Attachment #140843 - Flags: review?(mconnor)
This patch in its current form causes a regression. Bug 279074 was fixed when I
removed this patch.
Attachment #171456 - Flags: review?(mconnor)
Attached patch Updated Patch (obsolete) — Splinter Review
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! :)
Assignee: mconnor → gavin.sharp
Attachment #140843 - Attachment is obsolete: true
Attachment #171456 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #179242 - Flags: review?(mconnor)
Oh yeah, and most of this went in with bug 280438.
Whiteboard: [patch-r?]
Target Milestone: Future → Firefox1.1
Version: unspecified → Trunk
Attached patch Updated once again (obsolete) — Splinter Review
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).
Attachment #179242 - Attachment is obsolete: true
Attachment #184429 - Flags: review?(mconnor)
Attachment #179242 - Flags: review?(mconnor)
Attachment #184429 - Flags: review?(mconnor) → review+
Attachment #184429 - Flags: approval-aviary1.1a2?
Whiteboard: [patch-r?] → [patch-r+][checkin needed]
Attachment #184429 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Whiteboard: [patch-r+][checkin needed] → [patch-r+][checkin needed][a+]
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago15 years ago
Resolution: --- → FIXED
Whiteboard: [patch-r+][checkin needed][a+]
backed out, due to bug 297037.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patch v235 (obsolete) — Splinter Review
Last time, promise! :)

browser.js's onLocationChange needed to be tweaked to deal with the change.
Attachment #184429 - Attachment is obsolete: true
Attachment #195672 - Flags: review?(mconnor)
Status: REOPENED → ASSIGNED
Target Milestone: Firefox1.5 → ---
Whiteboard: [patch-r?]
Attachment #195672 - Attachment is obsolete: true
Attachment #195672 - Flags: review?(mconnor)
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
(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.
Whiteboard: [patch-r?]
Priority: P4 → P5
Target Milestone: --- → Future
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.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago13 years ago
Resolution: --- → FIXED
Depends on: 357776
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.
You need to log in before you can comment on or make changes to this bug.