Closed Bug 1172987 Opened 5 years ago Closed 5 years ago

Incorrect padding for various locales

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
Points:
5

Tracking

()

VERIFIED FIXED
Firefox 41
Iteration:
41.3 - Jun 29
Tracking Status
firefox40 --- fixed
firefox41 --- verified

People

(Reporter: emtwo, Assigned: emtwo)

References

(Blocks 1 open bug)

Details

(Whiteboard: .?)

Attachments

(2 files, 1 obsolete file)

No description provided.
Attached patch padding-fixes (obsolete) — Splinter Review
Addressed issues in bug 1164594 comment 13
Attachment #8617402 - Flags: review?(edilee)
Blocks: 1140185
Iteration: --- → 41.3 - Jun 29
Whiteboard: .?
Blocks: 1158859
Comment on attachment 8617402 [details] [diff] [review]
padding-fixes

>+++ b/browser/base/content/newtab/intro.js
>+          image = document.createElementNS(HTML_NAMESPACE, "div");
>           image.classList.add("newtab-intro-image-customize");
>+          let imageToCopy = document.getElementById("newtab-customize-panel").cloneNode(true);
>+          while (imageToCopy.firstChild) {
>+            image.appendChild(imageToCopy.firstChild);
>+          }
>+          image.setAttribute("id", "newtab-customize-panel");
Probably should add a comment for why we need to copy things over. Any good way for us to avoid having multiple nodes with the same id?

>+++ b/browser/base/content/newtab/newTab.css
>   transition: opacity .5s linear;
>   overflow: auto;
>   display: none;
>+
>+  text-align: center;
nit: extra space

>+#newtab-intro-text {
>+  text-align: left;
>+  right: 0px;
>+  max-width: 270px;
>+}
>+
> #newtab-intro-text,
> #newtab-intro-image {
>   height: 100%;
>-  width: 270px;
>   right: 0px;
>-  position: absolute;
>   font-size: 14px;
>   line-height: 20px;
>   margin-top: -12px;
>+  min-width: 290px;
Not sure if this will cause problems with max-width set to 270 then min-width 290. I seem to be running into issues if I resize the window smaller with the intro-text pushed down into the buttons.
Attachment #8617402 - Flags: review?(edilee)
Attachment #8617402 - Attachment is obsolete: true
Attachment #8621078 - Flags: review?(edilee)
Attachment #8621078 - Flags: review?(edilee) → review+
https://hg.mozilla.org/mozilla-central/rev/e7c0ddb5f948
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Duplicate of this bug: 1165394
Verified attachment 8621682 [details] bug 1165394 comment 9
Blocks: 1138818
Status: RESOLVED → VERIFIED
Points: --- → 5
Blocks: 1169237
Approval Request Comment

[Feature/regressing bug #]: 1138818 and 1158859 

[User impact if declined]: Padding issues in various locales. Screenshots in bug 1164594

[Describe test coverage new/current, TreeHerder]: verified on nightly. tested locally on aurora

[Risks and why]: low risk - mostly CSS changes

[String/UUID change made/needed]: N/A
Attachment #8622712 - Flags: approval-mozilla-aurora?
Comment on attachment 8622712 [details] [diff] [review]
[aurora] Padding fixes

Polish this new feature, taking it.
Attachment #8622712 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Looks like Marina landed this. Please don't forget to mark the bug accordingly in the future.
https://hg.mozilla.org/releases/mozilla-aurora/rev/9f5f01ca0c12
You need to log in before you can comment on or make changes to this bug.