Workaround for Bug 133527 no longer needed in components/alerts/resources/content/alert.js

RESOLVED FIXED in mozilla8

Status

()

Toolkit
XUL Widgets
--
trivial
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Philip Chee, Assigned: Javi Rueda)

Tracking

Trunk
mozilla8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
Bug 133527 introduced a workaround for bug 311557. See:
http://hg.mozilla.org/mozilla-central/annotate/c062731105cf/toolkit/components/alerts/resources/content/alert.js#l118
But subsequently bug 311557 was fixed.

So we should remove the workaround in alert.js and the comments in *stripe/global/alert/alerts.css

Noticed while working on Bug 652315.
(Assignee)

Comment 2

7 years ago
I am working on it. Don't know how to become "asignee".
(Assignee)

Comment 3

7 years ago
Created attachment 545261 [details] [diff] [review]
Patch for bug 652536

maybe some additional identation has been added or some blank-lines removed.
Attachment #545261 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #545261 - Flags: review? → review?(ventnor.bugzilla)

Comment 4

7 years ago
Comment on attachment 545261 [details] [diff] [review]
Patch for bug 652536


>   alertBox.orient = (gOrigin & NS_ALERT_HORIZONTAL) ? "vertical" : "horizontal";
> 
>-  // The above doesn't cause the labels in alertTextBox to reflow,
>-  // see bug 311557. As the theme's -moz-box-align css rule gets ignored,
>-  // we work around the bug by setting the align property.
>-  if (gOrigin & NS_ALERT_HORIZONTAL)
>-  {
>-    document.getElementById("alertTextBox").align = "center";
>-  }
>+  
> 
>   sizeToContent();

There's no need for 3 blank lines between code, 1 blank line will do.
r=me otherwise.
Attachment #545261 - Flags: review?(ventnor.bugzilla) → review+
(Assignee)

Comment 5

7 years ago
Created attachment 545341 [details] [diff] [review]
Patch with additional blank lines removed
Attachment #545341 - Flags: review?(ventnor.bugzilla)

Comment 6

7 years ago
Comment on attachment 545341 [details] [diff] [review]
Patch with additional blank lines removed

I already gave r+ with comments previously, so if you fix those comments, you don't have to ask me again and you can just check in the patch (or ask someone to do it by putting checkin-needed in keywords) :-)
Attachment #545341 - Flags: review?(ventnor.bugzilla) → review+
(Assignee)

Comment 7

7 years ago
Thanks Michael. This is my second patch sent, so I am not used to the process yet and a little afraid to disturb someone with new changes in the status.

Also, I am unable to change keywords in this bug as I am not the assignee of it. I assume no super-review is needed, as no "infrastructure" has been touched. So, if anybody assigns to me this bug, I would be able to change keywords to "check-in needed".

Before that, I am going to mark as "obsolete" my first patch sent in order to make easier for the "checker" the election of the right one.
(Assignee)

Updated

7 years ago
Attachment #545261 - Attachment is obsolete: true

Updated

7 years ago
Assignee: nobody → leofigueres
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/2ff6999222c2
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.