Closed
Bug 1078508
Opened 10 years ago
Closed 10 years ago
Design and implement doorhanger for unidentified security state (new tablet)
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 36
People
(Reporter: mcomella, Assigned: mcomella)
References
Details
Attachments
(3 files, 9 obsolete files)
Similar to desktop so when the not-identified globe is clicked (bug 1071267), we display a "This website does not supply identity information." doorhanger, or similar. Any thoughts on design and strings, Anthony?
Comment 1•10 years ago
|
||
Hmmm. I'll NI me to think about this more but for the time being, parity with Desktop does seem like a good place to start (in terms of string). I can't seem to recall what that looks like on Mobile ATM, have you got a screen shot handy perhaps?
Flags: needinfo?(alam)
Assignee | ||
Comment 2•10 years ago
|
||
Note that this image has some WIP patches and should be slightly different than the current build.
Comment 3•10 years ago
|
||
I'd like to give the top and bottom some more padding to free it up a bit (double what we have currently). But, for V1, let's aim for parity first and polish doorhangers as a part of a larger effort so we can include mobile devices too. How does that sound?
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
Assignee | ||
Comment 4•10 years ago
|
||
Sounds good to me. Just to be clear: I'm taking no action on V1 here.
Flags: needinfo?(michael.l.comella)
Comment 5•10 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #4) > Sounds good to me. > > Just to be clear: I'm taking no action on V1 here. Awesome. FYI https://bugzilla.mozilla.org/show_bug.cgi?id=1058818 is where the meta bug is filed to clean up doorhangers. :) I've also engaged in conversations with Desktop side so we can keep some consistency there as well (they're looking to possibly improve theirs).
Assignee | ||
Updated•10 years ago
|
Blocks: site-id-v2
Assignee | ||
Updated•10 years ago
|
Summary: Design and implement doorhanger for unidentified security state → Design and implement doorhanger for unidentified security state (new tablet)
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 6•10 years ago
|
||
Spoke IRL: antlam okayed us to implement something similar to desktop.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•10 years ago
|
||
Small cleanup: Params are easier to test and more predictable than using member variables.
Assignee | ||
Updated•10 years ago
|
Attachment #8521400 -
Flags: review?(bnicholson)
Assignee | ||
Comment 8•10 years ago
|
||
Changed something I didn't mean to.
Attachment #8521403 -
Flags: review?(bnicholson)
Assignee | ||
Updated•10 years ago
|
Attachment #8521400 -
Attachment is obsolete: true
Attachment #8521400 -
Flags: review?(bnicholson)
Assignee | ||
Comment 9•10 years ago
|
||
Not strictly necessary, but seemed a little more readable and safe (i.e. IDE's missing case warnings).
Attachment #8521406 -
Flags: review?(bnicholson)
Assignee | ||
Updated•10 years ago
|
Attachment #8521403 -
Attachment is obsolete: true
Attachment #8521403 -
Flags: review?(bnicholson)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8521406 [details] [diff] [review] Part 2: Change SiteIdentityPopup.updateUi to use a switch statement No longer relevant because rebase.
Attachment #8521406 -
Attachment is obsolete: true
Attachment #8521406 -
Flags: review?(bnicholson)
Assignee | ||
Updated•10 years ago
|
Attachment #8521420 -
Attachment is obsolete: true
Attachment #8521420 -
Flags: review?(bnicholson)
Assignee | ||
Comment 12•10 years ago
|
||
Attachment #8522923 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 13•10 years ago
|
||
Anthony, what do you think? ...vroom, vroom.
Attachment #8522924 -
Flags: feedback?(alam)
Assignee | ||
Comment 14•10 years ago
|
||
On comment 12: due to a bad hg qref error, I merged p1 w/ p2 - oops! Both are in the patch in comment 12 and still pretty readable. See comment 7 for motivation on taking the SiteIdentity parameter in updateIdentity.
Assignee | ||
Comment 15•10 years ago
|
||
Really, really bad with the qref these days.
Attachment #8522931 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8522923 -
Attachment is obsolete: true
Attachment #8522923 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 16•10 years ago
|
||
On about: pages, desktop shows "This is a secure Firefox site" or similar - we were showing "This site has an unindentified identity." or similar, which is misleading and bad. Part 2 fixes this by not showing a doorhanger. Really, really, really bad at qref, so this is included here. Added a followup for implementing the "This is a secure Firefox site", as per desktop. See bug 1099088.
Attachment #8522944 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8522931 -
Attachment is obsolete: true
Attachment #8522931 -
Flags: review?(lucasr.at.mozilla)
Comment 17•10 years ago
|
||
Comment on attachment 8522944 [details] [diff] [review] Add "unknown site identity" popup Review of attachment 8522944 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Just need the missing layout file to give r+. ::: mobile/android/base/locales/en-US/android_strings.dtd @@ +474,5 @@ > with that structure, consider a translation which ignores the preceding domain and > just addresses the organization to follow, e.g. "This site is run by " --> > <!ENTITY identity_run_by "which is run by"> > +<!ENTITY identity_no_info "This website does not supply identity information."> > +<!ENTITY identity_not_encrypted "Your connection to this website is not encrypted."> I assume we're just using the same strings than desktop? ::: mobile/android/base/resources/layout/site_identity.xml @@ +21,2 @@ > > + <include layout="@layout/site_identity_unknown" /> Where's this layout? Forgot to include in the patch? I was going to suggest stubbing this bug it's probably not worth it. ::: mobile/android/base/toolbar/SiteIdentityPopup.java @@ +101,5 @@ > + > + mIdentity.setVisibility(View.VISIBLE); > + mContent.setPadding(0, 0, 0, 0); > + > + final boolean isIdentityKnown = siteIdentity.getSecurityMode() != SecurityMode.UNKNOWN; nit: enclose expression with parens.
Attachment #8522944 -
Flags: review?(lucasr.at.mozilla) → feedback+
Assignee | ||
Updated•10 years ago
|
Attachment #8522944 -
Attachment is obsolete: true
Comment 19•10 years ago
|
||
Comment on attachment 8522958 [details] [diff] [review] Add "unknown site identity" popup Review of attachment 8522958 [details] [diff] [review]: ----------------------------------------------------------------- Yep.
Attachment #8522958 -
Flags: review?(lucasr.at.mozilla) → review+
Comment 20•10 years ago
|
||
Comment on attachment 8522924 [details]
Unknown site identity
Looks likes it's a good place to start! :) I'm happy with this for V1.
Attachment #8522924 -
Flags: feedback?(alam) → feedback+
Updated•10 years ago
|
Priority: -- → P1
Assignee | ||
Comment 21•10 years ago
|
||
Updated for nits.
Assignee | ||
Updated•10 years ago
|
Attachment #8522958 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8524852 -
Flags: review+
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #17) > I assume we're just using the same strings than desktop? Yep. > Where's this layout? Forgot to include in the patch? Added. > nit: enclose expression with parens. Done.
Assignee | ||
Comment 23•10 years ago
|
||
Apparently forgot some imports.
Assignee | ||
Updated•10 years ago
|
Attachment #8524852 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8524953 -
Flags: review+
Assignee | ||
Comment 24•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3ddd0d4aa067
Comment 25•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3ddd0d4aa067
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•