Closed
Bug 1257730
Opened 8 years ago
Closed 8 years ago
Crash with <a ping> and changing <base href>
Categories
(Core :: DOM: Navigation, defect)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: jruderman, Assigned: ckerschb)
References
Details
(Keywords: crash, regression, testcase)
Crash Data
Attachments
(4 files)
499 bytes,
text/html
|
Details | |
5.23 KB,
text/plain
|
Details | |
1.15 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
1.50 KB,
patch
|
RyanVM
:
review+
|
Details | Diff | Splinter Review |
Testcase requires: user_pref("browser.send_pings", true); Crashing line was added in: http://hg.mozilla.org/mozilla-central/rev/fa170734154a#l1.66
Reporter | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 3•8 years ago
|
||
Yeah, will fix that up.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(mozilla)
Assignee | ||
Comment 4•8 years ago
|
||
Thanks Jesse for the testcase. Jonas, in the old version there was a null check for the uri here [1], we should make sure we don't deref a nullptr in the new version as well. [1] http://hg.mozilla.org/mozilla-central/rev/fa170734154a#l1.15
Attachment #8732369 -
Flags: review?(jonas)
Assignee | ||
Updated•8 years ago
|
Blocks: 1192955
Keywords: regression
Attachment #8732369 -
Flags: review?(jonas) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 5•8 years ago
|
||
Any reason we aren't landing the test as a crashtest too?
Flags: needinfo?(mozilla)
Flags: in-testsuite?
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #5) > Any reason we aren't landing the test as a crashtest too? You are right. I'll convert the test into a crashtest and we should land that too. thanks!
Flags: needinfo?(mozilla)
Keywords: checkin-needed
Assignee | ||
Comment 7•8 years ago
|
||
Fails without the patch, but works correctly (no crash) with the patch. Ryan, to speed things up I think it's fine if you review the test, right?
Flags: needinfo?(ryanvm)
Attachment #8732425 -
Flags: review?(ryanvm)
Updated•8 years ago
|
Attachment #8732425 -
Flags: review?(ryanvm) → review+
Updated•8 years ago
|
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
Updated•8 years ago
|
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/integration/mozilla-inbound/rev/147b57e8c949 https://hg.mozilla.org/integration/mozilla-inbound/rev/10d8bc81030b
Keywords: checkin-needed
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/147b57e8c949 https://hg.mozilla.org/mozilla-central/rev/10d8bc81030b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•