Closed
Bug 777589
Opened 12 years ago
Closed 12 years ago
jstest: Remove a 2 line workaround to a 2 line test fix
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: terrence, Assigned: terrence)
References
(Blocks 1 open bug)
Details
(Whiteboard: [js:t])
Attachments
(1 file, 1 obsolete file)
3.43 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
Oddness in the extreme.
Attachment #645968 -
Flags: review?(sphink)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [js:t]
Comment 1•12 years ago
|
||
Comment on attachment 645968 [details] [diff] [review] v0 Review of attachment 645968 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/tests/js1_5/extensions/regress-50447-1.js @@ +102,5 @@ > var expectedLine = 114; > var expectedFileName = 'js1_5/extensions/regress-50447-1.js'; > if (typeof document == "undefined") > { > + expectedFileName = expectedFileName; I'm not going to look at this patch very closely, since it does seem rather odd and if everything works after it, I'm happy. But why not go ahead and finish simplifying this? if (typeof document != "undefined") { ...href.replace... } The "expectedFileName = expectedFileName" just makes me suspicious of bizarro magic that isn't happening.
Attachment #645968 -
Flags: review?(sphink) → review+
Assignee | ||
Comment 2•12 years ago
|
||
Well, I only looked at code that I know runs during shell tests so that it could get checked in with DONTBUILD. I'll see if there is more in that section that I can clean up before I commit it.
Assignee | ||
Comment 3•12 years ago
|
||
Oh, I see what you mean. I know I was feeling rather tired yesterday, but still, wow.
Assignee | ||
Comment 4•12 years ago
|
||
Oh yes, there was a reason that I kept the code that way: the test is checking that the exceptions it raises have the right line number. I fixed the line numbers for the shell, going to give it a try in the browser to make sure I didn't throw those off.
Attachment #645968 -
Attachment is obsolete: true
Attachment #646227 -
Flags: review+
Comment 5•12 years ago
|
||
Push backed out for bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/7a07ce25de7d
Assignee | ||
Comment 6•12 years ago
|
||
Green try run at: https://tbpl.mozilla.org/?tree=Try&rev=d7e9281d8857 Checked in at: https://hg.mozilla.org/integration/mozilla-inbound/rev/01b9cf7935be
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/01b9cf7935be
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•