Closed
Bug 1207107
Opened 9 years ago
Closed 9 years ago
Modernize the UI of aboutCertError.xhtml
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
People
(Reporter: past, Assigned: bgrins)
References
(Blocks 1 open bug)
Details
(Keywords: user-doc-needed, Whiteboard: [fxprivacy])
Attachments
(4 files, 7 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
text/x-review-board-request
|
dao
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details |
(deleted),
text/x-review-board-request
|
dao
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details |
(deleted),
text/x-review-board-request
|
dao
:
review+
|
Details |
aboutCertError.xhtml still contains Larry and is in dire need of a fresh coat of paint. Bram has an updated UI spec that can be found here: http://brampitoyo.github.io/fx-untrusted-connection/ Slight alterations for different error types can be found here: http://brampitoyo.github.io/fx-untrusted-connection/severe.xhtml http://brampitoyo.github.io/fx-untrusted-connection/rc4.xhtml This bug is about applying the common set of changes to the markup and stylesheets of aboutCertError.xhtml. Followup bugs will take care of the particular details of each error. This bug needs to implement the following: - Display the broken lock icon instead of Larry - Use the headline: “Your connection is not secure” - Use the following copy for the main message: “The owner of expired.badssl.com has configured their website improperly. To protect your information from being stolen, Firefox has not connected to this website.” - Add a “Learn more…” link that takes the user to the SUMO page: https://support.mozilla.org/en-US/kb/tls-error-reports - Add a “Return to Previous Page” button that does "history.back()" or goes to "about:home" - Add an “Advanced” button that reveals technical details about the error that are currently under the "Technical Details" section.
Updated•9 years ago
|
Flags: qe-verify?
Reporter | ||
Updated•9 years ago
|
Flags: qe-verify? → qe-verify+
Updated•9 years ago
|
Priority: P1 → P3
Updated•9 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Iteration: --- → 44.1 - Oct 5
Priority: P3 → P1
Updated•9 years ago
|
Iteration: 44.1 - Oct 5 → 44.2 - Oct 19
Assignee | ||
Comment 1•9 years ago
|
||
Literally just an import of http://brampitoyo.github.io/fx-untrusted-connection/. Still has hardcoded 'technical details' section, no behavior on the 'report errors' checkbox, and other issues.
Updated•9 years ago
|
Iteration: 44.2 - Oct 19 → 44.3 - Nov 2
Reporter | ||
Comment 2•9 years ago
|
||
I've removed a lot of the redundancies, but I think some of the JS is still not needed. Other things that remain are: 1. figure out why the hostname doesn't appear in bold 2. the Advanced button needs to be wider 3. verify that error reporting works 4. make sure that the override link appears in all the right cases and has the updated style I'll get back to it in the weekend or Monday morning.
Reporter | ||
Updated•9 years ago
|
Attachment #8671062 -
Attachment is obsolete: true
Assignee | ||
Comment 3•9 years ago
|
||
This fixes points 1 and 2 from Comment 2 (and is meant to be applied on top of the other patch). It also adds the beginnings of a simple test that we can hopefully enhance as we go. I'm starting to get a better idea of how the reporting and other neterror features work and have left a couple of comments in the patch. I wonder if we can cut scope in this bug by moving reporting and the override link into separate work and keeping the current exception popup (which is at parity at least with the current UI). If that were the case, I think we can still trim out stuff from the JS on this page that was imported from about:neterror that isn't used here. I'll leave this patch here, and we can discuss further on Monday if you get a chance to look at it.
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #3) > Created attachment 8678383 [details] [diff] [review] > changes-for-aboutcerterror.patch > > This fixes points 1 and 2 from Comment 2 (and is meant to be applied on top > of the other patch). It also adds the beginnings of a simple test that we > can hopefully enhance as we go. > > I'm starting to get a better idea of how the reporting and other neterror > features work and have left a couple of comments in the patch. I wonder if > we can cut scope in this bug by moving reporting and the override link into > separate work and keeping the current exception popup (which is at parity at > least with the current UI). If that were the case, I think we can still > trim out stuff from the JS on this page that was imported from > about:neterror that isn't used here. I'll leave this patch here, and we can > discuss further on Monday if you get a chance to look at it. Discussed this further, and the plan is to cut the 'report error' functionality from this bug (which means we don't need as much about:neterror stuff imported into the JS for this page). Also, we are planning to keep the 'add exception' modal workflow for now instead of the '(not secure) go to expired.badssl.com' link.
Reporter | ||
Comment 5•9 years ago
|
||
Didn't have much time for this today, but I've folded both patches into one and added a fallback for the "Return to previous page" button to go to about:home in case history is empty.
Reporter | ||
Updated•9 years ago
|
Attachment #8678259 -
Attachment is obsolete: true
Attachment #8678383 -
Attachment is obsolete: true
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to Panos Astithas [:past] from comment #5) > Created attachment 8678964 [details] [diff] [review] > Patch v4 > > Didn't have much time for this today, but I've folded both patches into one > and added a fallback for the "Return to previous page" button to go to > about:home in case history is empty. Alright, I'm going to cut out some functionality from this patch and fix up the test a bit
Assignee | ||
Comment 7•9 years ago
|
||
Separated the platform string changes out into another patch since it's not clear if we want to tackle this for v1
Assignee | ||
Comment 8•9 years ago
|
||
v5 - as far as I'm going to get today. This is a much smaller patch since it takes comment 4 into account and goes back to the modal exceptions button, along with taking away the error reporting feature. Hopefully the diff is easier to read now, too. I've expanded the test coverage a bit and put in stubs for extra test methods I'd like to have that will cover two special cases I see in the code. TODO: 1) Should 'learn more' link URL be localizable / stored outside of the HTML somewhere? Looks like similar pages aren't doing that so maybe not. 2) Expand test coverage - there are two particular things I'd like to check, and we should also have some basic integration testing to make sure that pressing 'advanced' works how you'd expect. There are already separate tests for the modal exceptions stuff so I think we are good there. 3) Review pass through on HTML / CSS to make sure that the import is clean (i.e. no extra elements / css rules that are unused). 4) Code cleanup.. There are things like the endsWith helper that aren't needed anymore (String.prototype.endsWith)
Attachment #8678964 -
Attachment is obsolete: true
Assignee | ||
Comment 9•9 years ago
|
||
I've got a try push as a sanity check for the current WIP: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3c6e4b77a0f
Comment 10•9 years ago
|
||
This bug blocks bug 1202488 and 1202489, but severe (non-overridable) errors and RC4 error will not use aboutCertError.xhtml. They will use aboutNetError.xhtml.
Reporter | ||
Updated•9 years ago
|
Attachment #8679131 -
Attachment is obsolete: true
Reporter | ||
Comment 12•9 years ago
|
||
New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c3d223368c0
Reporter | ||
Comment 14•9 years ago
|
||
If we are not landing attachment 8679125 [details] [diff] [review] yet, then we might want to move it to bug 1207146.
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to Panos Astithas [:past] from comment #14) > If we are not landing attachment 8679125 [details] [diff] [review] yet, then > we might want to move it to bug 1207146. Sounds good to me
Assignee | ||
Comment 16•9 years ago
|
||
Comment on attachment 8679435 [details] [diff] [review] about-cert-error-v6 Review of attachment 8679435 [details] [diff] [review]: ----------------------------------------------------------------- Dao, I see you've done reviews for aboutCertError in the past. Would you be able to review this? We are hoping to get these UI changes in for 44, so if you are not going to be able to look at it this week please let me know.
Attachment #8679435 -
Flags: review?(dao)
Comment 17•9 years ago
|
||
Comment on attachment 8679435 [details] [diff] [review] about-cert-error-v6 >--- a/browser/base/content/aboutcerterror/aboutCertError.xhtml >+++ b/browser/base/content/aboutcerterror/aboutCertError.xhtml >@@ -3,36 +3,39 @@ > <!DOCTYPE html [ > <!ENTITY % htmlDTD > PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" > "DTD/xhtml1-strict.dtd"> > %htmlDTD; > <!ENTITY % globalDTD > SYSTEM "chrome://global/locale/global.dtd"> > %globalDTD; >+ <!ENTITY % netErrorDTD >+ SYSTEM "chrome://global/locale/netError.dtd"> >+ %netErrorDTD; What netError.dtd strings are you using here and why? >- <link rel="icon" type="image/png" id="favicon" href="chrome://global/skin/icons/warning-16.png"/> >+ <link rel="icon" type="image/png" id="favicon" href="chrome://global/skin/icons/warning-16.png" /> Please avoid this change, the trailing space isn't useful. >+ function toggleVisibility(id) >+ { >+ var node = document.getElementById(id); >+ var toggle = { >+ '': 'inherit', >+ 'hidden': 'inherit', >+ 'inherit': 'hidden' >+ }; >+ node.style.visibility = toggle[node.style.visibility]; '': 'inherit' is wrong, and please use double quotes. How about: > node.style.visibility = node.style.visibility == "" ? "hidden" : ""; >+ // Get the hostname and add it to the panel >+ [...document.querySelectorAll('.hostname')].forEach(host => { >+ host.textContent = document.location.hostname; >+ }); for (let host of document.querySelectorAll(".hostname")) { host.textContent = document.location.hostname; } >+function waitForCertErrorLoad(browser) { >+ return new Promise(resolve => { >+ info("Waiting for AboutCertErrorLoad event"); >+ browser.addEventListener("AboutCertErrorLoad", function load() { >+ browser.removeEventListener("AboutCertErrorLoad", load, false, true); >+ resolve(); >+ }, false, true); >+ }); >+} Can you just use the standard DOMContentLoaded event and poke the document to make sure it's about:certerror? >+ is(advancedDiv.style.visibility, "hidden", "Advanced content should not be visible by default"); >+ is(advancedDiv.style.visibility, "inherit", "Advanced content should be visible by default"); >+ is(aP.style.visibility, "hidden", "Advanced content should not be visible by default"); use getComputedStyle > #errorTitle { >- -moz-margin-start: 80px; >+ background: url("chrome://browser/skin/cert-error.svg") left 0 no-repeat; rtl? >+ -moz-margin-start: -5em; >+ -moz-padding-start: 5em; please use margin-inline-start, padding-inline-start >+/* Pressing the retry button will cause the cursor to flicker from a pointer to >+ * not-allowed. Override the disabled cursor behaviour since we will never show >+ * the button disabled as the initial state. */ >+button:disabled { > cursor: pointer; > } Can we just remove <http://hg.mozilla.org/mozilla-central/annotate/fc706d376f06/toolkit/themes/shared/in-content/common.inc.css#l213>? Disabled buttons using the not-allowed cursor is unusual and therefore seems more distracting than useful. >+#advancedButton { >+ float: right; >+ min-width: 150px; >+} This is correct for rtl too? >+/* Advanced section is hidden via inline styles until the link is clicked */ >+#advancedPanel { >+ background-color: white; Should probably specify a text color here rather than counting on the inherited color working on white. >+ border: 1px lightgray solid; >+ /* Don't use top padding because the default p style has top padding, and it >+ * makes the overall div look uneven */ >+ padding: 0 12px 12px 12px; >+ box-shadow: 0 0 4px #ddd; >+ font-size: 0.9em; >+ margin-top: 10px; >+ padding-bottom: 10px; padding: 0 12px 10px;
Attachment #8679435 -
Flags: review?(dao) → review-
Assignee | ||
Comment 18•9 years ago
|
||
Apparently the merge day is tomorrow, so we will need to land the new strings ASAP if we want to get it in for 44
Assignee | ||
Comment 19•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #17) > Comment on attachment 8679435 [details] [diff] [review] > about-cert-error-v6 > > >--- a/browser/base/content/aboutcerterror/aboutCertError.xhtml > >+++ b/browser/base/content/aboutcerterror/aboutCertError.xhtml > >@@ -3,36 +3,39 @@ > > <!DOCTYPE html [ > > <!ENTITY % htmlDTD > > PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" > > "DTD/xhtml1-strict.dtd"> > > %htmlDTD; > > <!ENTITY % globalDTD > > SYSTEM "chrome://global/locale/global.dtd"> > > %globalDTD; > >+ <!ENTITY % netErrorDTD > >+ SYSTEM "chrome://global/locale/netError.dtd"> > >+ %netErrorDTD; > > What netError.dtd strings are you using here and why? > It's using the errorReporting.learnMore and weakCryptoAdvanced.title but we could add new strings into the cert error dtd for these and remove the reference to netError.dtd > >- <link rel="icon" type="image/png" id="favicon" href="chrome://global/skin/icons/warning-16.png"/> > >+ <link rel="icon" type="image/png" id="favicon" href="chrome://global/skin/icons/warning-16.png" /> > > Please avoid this change, the trailing space isn't useful. > > >+ function toggleVisibility(id) > >+ { > >+ var node = document.getElementById(id); > >+ var toggle = { > >+ '': 'inherit', > >+ 'hidden': 'inherit', > >+ 'inherit': 'hidden' > >+ }; > >+ node.style.visibility = toggle[node.style.visibility]; > > '': 'inherit' is wrong, and please use double quotes. How about: > > > node.style.visibility = node.style.visibility == "" ? "hidden" : ""; > Updated, this was just copied from the mockup but the proposal is better > >+ // Get the hostname and add it to the panel > >+ [...document.querySelectorAll('.hostname')].forEach(host => { > >+ host.textContent = document.location.hostname; > >+ }); > > for (let host of document.querySelectorAll(".hostname")) { > host.textContent = document.location.hostname; > } Updated > > >+function waitForCertErrorLoad(browser) { > >+ return new Promise(resolve => { > >+ info("Waiting for AboutCertErrorLoad event"); > >+ browser.addEventListener("AboutCertErrorLoad", function load() { > >+ browser.removeEventListener("AboutCertErrorLoad", load, false, true); > >+ resolve(); > >+ }, false, true); > >+ }); > >+} > > Can you just use the standard DOMContentLoaded event and poke the document > to make sure it's about:certerror? Pretty sure the normal load event don't fire for these error pages, which is why aboutNetError dispatches an AboutNetErrorLoad event. When I tried to just wait for load in the test it never fired. I'm not sure about DOMContentLoaded and if the test could listen for that. > >+ is(advancedDiv.style.visibility, "hidden", "Advanced content should not be visible by default"); > > >+ is(advancedDiv.style.visibility, "inherit", "Advanced content should be visible by default"); > > >+ is(aP.style.visibility, "hidden", "Advanced content should not be visible by default"); > > use getComputedStyle > Switched to is_element_hidden / is_element_visible > > #errorTitle { > >- -moz-margin-start: 80px; > >+ background: url("chrome://browser/skin/cert-error.svg") left 0 no-repeat; > > rtl? > > >+ -moz-margin-start: -5em; > >+ -moz-padding-start: 5em; > > please use margin-inline-start, padding-inline-start > > >+/* Pressing the retry button will cause the cursor to flicker from a pointer to > >+ * not-allowed. Override the disabled cursor behaviour since we will never show > >+ * the button disabled as the initial state. */ > >+button:disabled { > > cursor: pointer; > > } > > Can we just remove > <http://hg.mozilla.org/mozilla-central/annotate/fc706d376f06/toolkit/themes/ > shared/in-content/common.inc.css#l213>? > > Disabled buttons using the not-allowed cursor is unusual and therefore seems > more distracting than useful. > Fine with me, although this would affect all in content pages and that targets more than just buttons (colorpickers and menulists also). Since we are attempting to land this late in the cycle maybe it'd be safest to keep it as-is and file a follow up to make that change. > >+#advancedButton { > >+ float: right; > >+ min-width: 150px; > >+} > > This is correct for rtl too? > > >+/* Advanced section is hidden via inline styles until the link is clicked */ > >+#advancedPanel { > >+ background-color: white; > > Should probably specify a text color here rather than counting on the > inherited color working on white. > > >+ border: 1px lightgray solid; > >+ /* Don't use top padding because the default p style has top padding, and it > >+ * makes the overall div look uneven */ > >+ padding: 0 12px 12px 12px; > >+ box-shadow: 0 0 4px #ddd; > >+ font-size: 0.9em; > >+ margin-top: 10px; > >+ padding-bottom: 10px; > > padding: 0 12px 10px; I'll update the CSS
Assignee | ||
Comment 20•9 years ago
|
||
Bug 1207107 - Strings for aboutCertError design update;r=dao
Attachment #8680298 -
Flags: review?(dao)
Assignee | ||
Comment 21•9 years ago
|
||
Bug 1207107 - Design update for aboutCertError;r=dao
Attachment #8680299 -
Flags: review?(dao)
Assignee | ||
Comment 22•9 years ago
|
||
Bug 1207107 - Remove old strings for aboutCertError;r=dao
Attachment #8680300 -
Flags: review?(dao)
Assignee | ||
Comment 23•9 years ago
|
||
Thanks for the review Dao! I've updated the patch as you suggested, except where there are other notes. The first patch is meant to land before the merge tomorrow. Part 2 could be uplifted, and part 3 (string removal) can land after merge and ride the train. Also, I believe that we can use DOMContentLoaded instead of the AboutCertErrorLoad event, so I'll update that and push a change.
Assignee | ||
Comment 24•9 years ago
|
||
New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb63a4174d9e
Assignee | ||
Comment 25•9 years ago
|
||
Comment on attachment 8680299 [details] MozReview Request: Bug 1207107 - Design update for aboutCertError;r=dao Bug 1207107 - Design update for aboutCertError;r=dao
Assignee | ||
Comment 26•9 years ago
|
||
Comment on attachment 8680300 [details] MozReview Request: Bug 1207107 - Remove old strings for aboutCertError;r=dao Bug 1207107 - Remove old strings for aboutCertError;r=dao
Assignee | ||
Updated•9 years ago
|
Attachment #8679125 -
Attachment is obsolete: true
Comment 27•9 years ago
|
||
Comment on attachment 8680298 [details] MozReview Request: Bug 1207107 - Strings for aboutCertError design update;r=dao Please add a l10n note for certerror.introPara that explains #1 and the span element. I'm actually not even sure what #1 is good for; can we remove it and just use <span class='hostname'/>?
Attachment #8680298 -
Flags: review?(dao) → review+
Updated•9 years ago
|
Attachment #8680300 -
Flags: review?(dao) → review+
Reporter | ||
Comment 29•9 years ago
|
||
I landed the string update patch after removing the redundant #1 bit.
Comment 30•9 years ago
|
||
Comment on attachment 8680299 [details] MozReview Request: Bug 1207107 - Design update for aboutCertError;r=dao (In reply to Brian Grinstead [:bgrins] from comment #19) > > >+/* Pressing the retry button will cause the cursor to flicker from a pointer to > > >+ * not-allowed. Override the disabled cursor behaviour since we will never show > > >+ * the button disabled as the initial state. */ > > >+button:disabled { > > > cursor: pointer; > > > } > > > > Can we just remove > > <http://hg.mozilla.org/mozilla-central/annotate/fc706d376f06/toolkit/themes/ > > shared/in-content/common.inc.css#l213>? > > > > Disabled buttons using the not-allowed cursor is unusual and therefore seems > > more distracting than useful. > > > > Fine with me, although this would affect all in content pages and that > targets more than just buttons (colorpickers and menulists also). Since we > are attempting to land this late in the cycle maybe it'd be safest to keep > it as-is and file a follow up to make that change. Can you please file the bug and add a comment to the rule with a reference to that bug? > <button id="returnButton" autocomplete="off" onclick="goBack(this);" autofocus="true" style="margin-left: 0;">&certerror.returnToPreviousPage.label;</button> What's style="margin-left: 0;" doing here?
Attachment #8680299 -
Flags: review?(dao) → review+
Assignee | ||
Comment 31•9 years ago
|
||
Thanks for the reviews! I moved the inline margin-left to the CSS file and changed it to margin-inline-start and also filed Bug 1219861 to handle the button styling for in-content pages Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4cbd2b7eaf70
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 32•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/511f709bd650 https://hg.mozilla.org/integration/fx-team/rev/8e4fa465e2c6
Assignee | ||
Updated•9 years ago
|
Attachment #8679435 -
Attachment is obsolete: true
Assignee | ||
Comment 34•9 years ago
|
||
Comment on attachment 8680298 [details] MozReview Request: Bug 1207107 - Strings for aboutCertError design update;r=dao Approval Request Comment [Feature/regressing bug #]: New design for bad certificate error page [User impact if declined]: The new about:certerror won't ship in 44 [Describe test coverage new/current, TreeHerder]: Tests are in part 2 [Risks and why]: This is just the string change, see part 2 [String/UUID change made/needed]: This is the string change for the feature, so adding new strings to aboutCertError.dtd
Attachment #8680298 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 35•9 years ago
|
||
Comment on attachment 8680299 [details] MozReview Request: Bug 1207107 - Design update for aboutCertError;r=dao Approval Request Comment [Feature/regressing bug #]: New design for bad certificate error page [User impact if declined]: The new about:certerror won't ship in 44 [Describe test coverage new/current, TreeHerder]: Added a test for about:certerror [Risks and why]: It's a new design, so there's a chance for visual regressions. The strings and design has been through product reviews but the implementation could have bugs. It's a fairly shallow visual design change and it's not replacing the cert exception workflow so that at least limits the surface area [String/UUID change made/needed]: None here, see part 1
Attachment #8680299 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 36•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/511f709bd650 https://hg.mozilla.org/mozilla-central/rev/8e4fa465e2c6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Comment 37•9 years ago
|
||
Comment on attachment 8680298 [details] MozReview Request: Bug 1207107 - Strings for aboutCertError design update;r=dao Cool new design, taking it. Should be in the first 44 devedition release.
Attachment #8680298 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 38•9 years ago
|
||
Comment on attachment 8680299 [details] MozReview Request: Bug 1207107 - Design update for aboutCertError;r=dao Cool new design, taking it. Should be in the first 44 devedition release.
Attachment #8680299 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Updated•9 years ago
|
Keywords: user-doc-needed
Comment 39•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/bacfca882a0c https://hg.mozilla.org/releases/mozilla-aurora/rev/00fe0e2bbdbb
Comment 40•9 years ago
|
||
https://reviewboard.mozilla.org/r/23609/#review21317 Kinda makes me sad we aren't using the info-pages.css stylesheet that was created for this kind of page...
Comment 41•9 years ago
|
||
https://reviewboard.mozilla.org/r/23609/#review21321 ::: browser/themes/shared/aboutCertError.css:71 (Diff revision 2) > + background-color: var(--in-content-primary-button-background-hover) !important; Additionally, there's already a class name available for blue buttons. It's "primary". You can use it to avoid style duplication. ::: browser/themes/shared/incontent-icons/cert-error.svg:4 (Diff revision 2) > +<svg version="1.1" This SVG needs cleanup, I wish we could set up a tool to clean up our SVGs (SVGO seems like a great candidate).
Comment 42•9 years ago
|
||
> ::: browser/themes/shared/incontent-icons/cert-error.svg:4 > (Diff revision 2) > > +<svg version="1.1" > > This SVG needs cleanup, I wish we could set up a tool to clean up our SVGs > (SVGO seems like a great candidate). Filed bug 1220483.
Updated•9 years ago
|
QA Contact: paul.silaghi
Comment 43•9 years ago
|
||
The new UI looks ok, marking verified fixed considering the follow-up bugs. 45.0a1 (2015-11-02), 44.0a2 (2015-11-02) win 7.
Comment 44•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/bbc15862ad55 https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/511f709bd650 https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/8e4fa465e2c6
status-b2g-v2.5:
--- → fixed
Comment 45•9 years ago
|
||
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
Comment 46•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/bacfca882a0c https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/00fe0e2bbdbb
status-b2g-v2.5:
--- → fixed
Depends on: 1229515
Updated•9 years ago
|
status-b2g-v2.5:
fixed → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•