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

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Tabbed Browser
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

({fixed-seamonkey2.0.3})

Trunk
seamonkey2.1a1
fixed-seamonkey2.0.3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
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...
}
(Assignee)

Updated

7 years ago
(Assignee)

Updated

7 years ago
Depends on: 504344
(Assignee)

Comment 1

7 years ago
Created attachment 419288 [details] [diff] [review]
(Av1) tabbrowser.xml: check for empty URI too

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 2

7 years ago
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
(Assignee)

Comment 3

7 years ago
Created attachment 419295 [details] [diff] [review]
(Av1a) tabbrowser.xml: check for |!aURI| too
[Checkin: Comment 5 & 11]

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 4

7 years ago
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+
(Assignee)

Comment 5

7 years ago
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]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

7 years ago
(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'.)

Comment 7

7 years ago
> (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)?
(Assignee)

Comment 9

7 years ago
(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...
(Assignee)

Comment 10

7 years ago
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?
(Assignee)

Updated

7 years ago
Blocks: 534647

Updated

7 years ago
Attachment #419295 - Flags: approval-seamonkey2.0.3? → approval-seamonkey2.0.3+
(Assignee)

Comment 11

7 years ago
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]
(Assignee)

Updated

7 years ago
Keywords: fixed-seamonkey2.0.3
You need to log in before you can comment on or make changes to this bug.