nsMixedContentBlocker::AsyncOnChannelRedirect should return an error if there is no loadinfo

RESOLVED FIXED in Firefox 67

Status

()

defect
P2
normal
RESOLVED FIXED
5 years ago
6 months ago

People

(Reporter: tanvi, Assigned: ckerschb)

Tracking

(Blocks 2 bugs)

35 Branch
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 attachment, 1 obsolete attachment)

Once we are confident that all channels should have a loadinfo, we should return an error when a redirect request that goes through Mixed Content Blocker doesn't have loadinfo.

Depends on bug 418354 landing and js callers setting loadInfo on channel creation.
Component: Security → DOM: Security
Whiteboard: [domsecurity-backlog]
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [domsecurity-backlog] → [domsecurity-active]

Still having trouble with my lando account - can someone check that bug in for me please - thanks!

Keywords: checkin-needed

I've received the following error when trying to land this:
"ERROR **************************** remote: Rev 5050856d851c needs "Bug N" or "No bug" in the commit message. remote: Christoph Kerschbaumer <ckerschb@christophkerschbaumer.com> remote: Summary: r=jkt remote: Bug 1077201: No need to return if there is no loadinfo in the mixed content blocker - we have loadinfo everywhere. r=jkt remote: remote: Differential Revision: https://phabricator.services.mozilla.com/D19658 remote: ************************************************************* remote: remote: remote: transaction abort! remote: rollback completed remote: pretxnchangegroup.c_commitmessage hook failed abort: push failed on remote"

;ckerschb Can you please take a look?

Flags: needinfo?(ckerschb)
Keywords: checkin-needed

I don't get phabricator review requests. So I downloaded the raw diff from phab (since I am on a different machine). Updated the message (using hg amend -m) and did an arc diff. Now it creates a new changeset with a new review request. Shouldn't it just update the old one?

What am I doing wrong? Sorry for the additional review - can you rubber stamp again so I can land this one?

Flags: needinfo?(ckerschb) → needinfo?(jkt)
Attachment #9043798 - Attachment is obsolete: true

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #6)

What am I doing wrong? Sorry for the additional review - can you rubber stamp again so I can land this one?

arc diff --update D19658

I still need to get used to phabricator - I think it works now.

Setting the checkin-keyword again

Flags: needinfo?(jkt)
Keywords: checkin-needed

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #7)

The patch failed to land again for the same issue in https://bugzilla.mozilla.org/show_bug.cgi?id=1077201#c4 .
Could you please remove "summary" from the commit message and then set it back to checkin-needed?
Thank you.

Flags: needinfo?(ckerschb)
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f50b598aae6
No need to return if there is no loadinfo in the mixed content blocker - we have loadinfo everywhere. r=jkt

(In reply to Natalia Csoregi [:nataliaCs] from comment #8)

The patch failed to land again for the same issue in https://bugzilla.mozilla.org/show_bug.cgi?id=1077201#c4 .
Could you please remove "summary" from the commit message and then set it back to checkin-needed?
Thank you.

Ah, still have to get used to phab - pushed it myself now - thanks.

Flags: needinfo?(ckerschb)
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.