Closed
Bug 1059600
Opened 10 years ago
Closed 9 years ago
New tabs for javascript: links don't have a title
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
People
(Reporter: bnicholson, Assigned: dao)
References
()
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
2.28 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
STR: 1) Go to data:text/html,<a href="javascript:void()">foo</a> 2) ctrl+click the link Previously, this would open a tab with the JS URI as the title. Now, it simply opens a tab with no title, resulting in an empty space in the tabs panel that looks broken. Regression range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a6d4659e1031&tochange=89cb4420380d MattN thinks this is from bug 1017156, so blocking that.
Reporter | ||
Updated•10 years ago
|
status-firefox32:
--- → unaffected
status-firefox33:
--- → affected
status-firefox34:
--- → affected
Updated•10 years ago
|
Flags: qe-verify?
Flags: firefox-backlog+
Updated•10 years ago
|
Updated•10 years ago
|
Points: --- → 2
Flags: qe-verify? → qe-verify+
Comment 2•10 years ago
|
||
Marco, can you add this to the prioritized list for the next iteration? Thanks!
Flags: needinfo?(mmucci)
Updated•9 years ago
|
Assignee: nobody → dao
Status: NEW → ASSIGNED
Iteration: --- → 37.1
Assignee | ||
Comment 4•9 years ago
|
||
Trivial stop-gap solution. This does not fix bug 1085197. That will be more involved.
Attachment #8531702 -
Flags: review?(gijskruitbosch+bugs)
Comment 5•9 years ago
|
||
Comment on attachment 8531702 [details] [diff] [review] patch Review of attachment 8531702 [details] [diff] [review]: ----------------------------------------------------------------- Is this better, in the end, than just backing out 1017156 and fixing 1085197 instead? ::: browser/base/content/tabbrowser.xml @@ +1658,5 @@ > // We've waited until the tab is in the DOM to set the label. This > // allows the TabLabelModified event to be properly dispatched. > if (!aURI || isBlankPageURL(aURI)) { > t.label = this.mStringBundle.getString("tabs.emptyTabTitle"); > + } else if (aURI.toLowerCase().startsWith("javascript:")) { Assuming you think the answer is 'no', should we do this for any other protocols, offhand?
Attachment #8531702 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #5) > Comment on attachment 8531702 [details] [diff] [review] > patch > > Review of attachment 8531702 [details] [diff] [review]: > ----------------------------------------------------------------- > > Is this better, in the end, than just backing out 1017156 and fixing 1085197 > instead? Well, bug 1085197 isn't an issue without bug 1017156, so this question doesn't make much sense to me. Anyway, I don't think bug 1017156 should be backed out because of bug 1085197. > ::: browser/base/content/tabbrowser.xml > @@ +1658,5 @@ > > // We've waited until the tab is in the DOM to set the label. This > > // allows the TabLabelModified event to be properly dispatched. > > if (!aURI || isBlankPageURL(aURI)) { > > t.label = this.mStringBundle.getString("tabs.emptyTabTitle"); > > + } else if (aURI.toLowerCase().startsWith("javascript:")) { > > Assuming you think the answer is 'no', should we do this for any other > protocols, offhand? Not that I know.
Comment 7•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #6) > (In reply to :Gijs Kruitbosch from comment #5) > > Comment on attachment 8531702 [details] [diff] [review] > > patch > > > > Review of attachment 8531702 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > Is this better, in the end, than just backing out 1017156 and fixing 1085197 > > instead? > > Well, bug 1085197 isn't an issue without bug 1017156, so this question > doesn't make much sense to me. Anyway, I don't think bug 1017156 should be > backed out because of bug 1085197. Right, I meant backing out bug 1017156 until we can reland it with both this and your suggestion in the last comment on bug 1085197 fixed. Anyway, if you think bug 1085197 isn't severe enough to bother backing anything out, then carry on. :-)
Assignee | ||
Comment 8•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/5d5425acf620
Comment 9•9 years ago
|
||
Not obvious enough for me to see why, but https://treeherder.mozilla.org/logviewer.html#?job_id=1355406&repo=fx-team, "browser_bug906190.js | uncaught exception - TypeError: aURI.toLowerCase is not a function at chrome://browser/content/tabbrowser.xml:1658". Backed out in https://hg.mozilla.org/integration/fx-team/rev/cb8849c52508.
Assignee | ||
Comment 10•9 years ago
|
||
browser_bug906190.js is off the charts. I don't know how this ever worked. https://hg.mozilla.org/integration/fx-team/rev/9a2c20e8fe59
Attachment #8531702 -
Attachment is obsolete: true
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9a2c20e8fe59
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Comment 12•9 years ago
|
||
I was able to reproduce this issue on Firefox 34.0a1 (2014-08-27) using Windows 7 x64. Verified fixed on Latest Firefox 37.0a1 (2014-12-07) with e10s enabled and disabled using Windows 7 x64, Ubuntu 14.04 x86 and Mac OSX 10.10.
Status: RESOLVED → VERIFIED
status-firefox37:
--- → verified
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8532566 [details] [diff] [review] patch, browser_bug906190.js fixed Approval Request Comment [Feature/regressing bug #]: bug 1017156 [User impact if declined]: see comment 0 [Describe test coverage new/current, TBPL]: this particular edge case doesn't have tests [Risks and why]: conservative fix, low risk [String/UUID change made/needed]: none
Attachment #8532566 -
Flags: approval-mozilla-beta?
Attachment #8532566 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8532566 -
Flags: approval-mozilla-beta?
Attachment #8532566 -
Flags: approval-mozilla-beta+
Attachment #8532566 -
Flags: approval-mozilla-aurora?
Attachment #8532566 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 14•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/cc3e1b1f2dba https://hg.mozilla.org/releases/mozilla-beta/rev/e3c4d97a574d
status-firefox35:
--- → fixed
status-firefox36:
--- → fixed
Updated•9 years ago
|
QA Contact: vasilica.mihasca
Comment 15•9 years ago
|
||
Verified fixed on Latest Firefox 36.0a2 (2014-12-11) and Firefox 35 Beta 3 (20141211142524) using Windows 7 x64, Ubuntu 14.04 x86 and Mac OSX 10.9.5
You need to log in
before you can comment on or make changes to this bug.
Description
•