Closed Bug 536940 Opened 10 years ago Closed 10 years ago

Port |Bug 468053 - gBrowser.addTab not treat null/undefined Uri as blank tab leading to extra work at least when restoring session| to SeaMonkey

Categories

(SeaMonkey :: Tabbed Browser, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1a1

People

(Reporter: sgautherie, Assigned: sgautherie)

References

()

Details

(Keywords: fixed-seamonkey2.0.3)

Attachments

(1 file, 1 obsolete file)

Bug 535907 comment 12:
{
neil@parkwaycc.co.uk      2009-12-20 13:47:31 PST

>     gBrowser.addTab().linkedBrowser.stop();
Note that our addTab doesn't correctly check for a missing argument...
}
Depends on: 504344
Please, double-check the 2 first changes which I just assumed needed to be fixed too.

Checking if/which tests need to be updated will have to wait until bug 504344 is fixed.
(I tried to run some test suites locally recently but they have too many other failures...)
Attachment #419288 - Flags: review?(neil)
Comment on attachment 419288 [details] [diff] [review]
(Av1) tabbrowser.xml: check for empty URI too

>-                    aLocation.spec != "about:blank")
>+                    (aLocation.spec && aLocation.spec != "about:blank"))

>-            if (aURI == "about:blank")
>+            if (!aURI || aURI == "about:blank")
As far as I know it's not possible for a URI to have an empty spec so we don't need these changes.

>-            var blank = (aURI == "about:blank");
>+            let blank = !aURI || aURI == "about:blank";
Mumble mumble var mumble mumble
Av1, with comment 2 suggestion(s).

(In reply to comment #2)

> As far as I know it's not possible for a URI to have an empty spec so we don't
> need these changes.

(Thanks, I just wanted to be sure I wasn't missing something.)

> Mumble mumble var mumble mumble

Well, what do you think is wrong with 'let'?
Attachment #419288 - Attachment is obsolete: true
Attachment #419295 - Flags: review?(neil)
Attachment #419288 - Flags: review?(neil)
Comment on attachment 419295 [details] [diff] [review]
(Av1a) tabbrowser.xml: check for |!aURI| too
[Checkin: Comment 5 & 11]

Here it's no help at all, but feel free to use let in the places that need it, which are few and far between.
Attachment #419295 - Flags: review?(neil) → review+
Comment on attachment 419295 [details] [diff] [review]
(Av1a) tabbrowser.xml: check for |!aURI| too
[Checkin: Comment 5 & 11]


http://hg.mozilla.org/comm-central/rev/2ab0fbab5907
Attachment #419295 - Attachment description: (Av1a) tabbrowser.xml: check for |!aURI| too → (Av1a) tabbrowser.xml: check for |!aURI| too [Checkin: Comment 5]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to comment #4)
> (From update of attachment 419295 [details] [diff] [review])
> Here it's no help at all, but feel free to use let in the places that need it,
> which are few and far between.

(Hum, I thought the new goal would be more like 'always use let, unless var is needed'.)
> (Hum, I thought the new goal would be more like 'always use let, unless var is
> needed'.)
Only in Firefox. In Suite it's always use var except when using let makes sense.
I see this is FIXED but apparently only on Trunk (Sm 2.1.x). Any idea if this fix is going to be ported to comm-1.9.1 (Sm 2.0.x)?
(In reply to comment #8)
> Any idea if this fix is going to be ported to comm-1.9.1 (Sm 2.0.x)?

It won't (though it's trivial), unless someone writes why it could pass approval...
Comment on attachment 419295 [details] [diff] [review]
(Av1a) tabbrowser.xml: check for |!aURI| too
[Checkin: Comment 5 & 11]

"approval-seamonkey2.0.3=?":
No risk. May help/fix bug 534647 (for example)...
Attachment #419295 - Flags: approval-seamonkey2.0.3?
Blocks: 534647
Attachment #419295 - Flags: approval-seamonkey2.0.3? → approval-seamonkey2.0.3+
Comment on attachment 419295 [details] [diff] [review]
(Av1a) tabbrowser.xml: check for |!aURI| too
[Checkin: Comment 5 & 11]


http://hg.mozilla.org/releases/comm-1.9.1/rev/392e4ef71e67
Attachment #419295 - Attachment description: (Av1a) tabbrowser.xml: check for |!aURI| too [Checkin: Comment 5] → (Av1a) tabbrowser.xml: check for |!aURI| too [Checkin: Comment 5 & 11]
You need to log in before you can comment on or make changes to this bug.