Closed Bug 1511834 Opened 2 years ago Closed 1 year ago

Site Information header in reader mode looks ugly


(Firefox :: Site Identity, defect, P3)




Firefox 67
Tracking Status
firefox67 --- fixed


(Reporter: ehsan, Assigned: ns19041997, Mentored)




(3 files)

Attached image Screenshot
See the screenshot, it shows the about:reader?url=foo URL...
Assignee: nobody → 1991manish.kumar
(In reply to Manish [:manishkk] from comment #1)
> these lines are code related to this bug?
> siteIdentity.js#763,779

Line 763 doesn't seem related, as about:reader does not have a EV certificate (hence it does not match the this._isEV check).
What Tim said!
Flags: needinfo?(ehsan)
Attached image no ugly header.png


but same URL working fine for me.

Attachment #9035803 - Flags: review?(ehsan)

(In reply to Manish [:manishkk] from comment #4)

Created attachment 9035803 [details]
no ugly header.png


but same URL working fine for me.

You need to click the reader icon in the URL bar (the one that looks like a page).

Attachment #9035803 - Flags: review?(ehsan)
Assignee: 1991manish.kumar → nobody

Some more info on how to solve this bug:

This is how the URL bar figures out the right url to display from an about:reader url:

We should do the same thing in refreshIdentityPopup, e.g. here:

It would also be great to have a test for this.

I would like to work on this.
Could this bug be assigned to me?

Yup, thank you!

Assignee: nobody → ns19041997

Hi, Johann,
I have made a patch regarding the site information header issue.
I also saw the files browser_mixed_content_cert_override.js and browser_check_identity_state.js tests where refreshIdentityPopup() is used.
Could you guide me as to where I should write a test for this.

browser_check_identity_state.js is pretty close to what you want to do, it loads different sites like and checks their security indicators.

You might want to do the same for some URL like about:reader?url=, but then we also would like to verify that identity-popup-mainView-panel-header-span contains the right URL (and doesn't contain about:reader).

Does that make sense?


Hello Johann,
I have made the requested changes.
Please review.

Hi Neha, thanks for the great contribution.

Before landing this, it would be great if you could do a Try Push. Try is basically Continuous Integration for Firefox, making sure that tests are passing for your patch.

Getting access is outlined here:

You will need to file a bug and need someone to vouch for you. I'd be happy to do that if you CC me on that bug.

Afterwards, make sure you perform the steps outlined in and (most importantly) in in order to get access on your local command line.

When you have access, to schedule a try run, you can use the handy ./mach try command. A possible syntax for this bug could be:

./mach try -b do -p win64,linux64,macosx64 -u mochitests -t none

(runs all mochitests on Windows, OSX and Linux)

Let me know if you need any help with that!

Hi Johann,
I got the access and scheduled a try run.
It has completed.
Please review.

Great, thanks, but I would need the link to the try run to review it :)

Based on searching treeherder for your email address I think it's this one, right?

That looks great to me, feel free to set checkin-needed now!

Hello Johann,
yes, this is the one I submitted!

Could you guide me as to how to set the checkin-needed option for this?
I went and searched the treeherder( page, but couldn't figure out how to set it.


You can do it in this bug, just select "Edit Bug" and modify the "keywords" field to include "checkin-needed" (it should autocomplete it for you).

Keywords: checkin-needed

I have made the required changes.
Please review.

Pushed by
Improving site Information header in reader mode r=johannh

Keywords: checkin-needed
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.