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

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Philip Chee]

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.

Comment 1

[Philip Chee]

> http://hg.mozilla.org/mozilla-central/annotate/c062731105cf/toolkit/components/alerts/resources/content/alert.js#l118
I meant line 130 of course.
http://hg.mozilla.org/mozilla-central/annotate/c062731105cf/toolkit/components/alerts/resources/content/alert.js#l130

Comment 2

[Javi Rueda :javirid]

I am working on it. Don't know how to become "asignee".

Comment 3

[Javi Rueda :javirid]

Attachment: Patch for bug 652536

maybe some additional identation has been added or some blank-lines removed.

Comment 4

[Michael Ventnor]

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.

Comment 5

[Javi Rueda :javirid]

Attachment: Patch with additional blank lines removed

Comment 6

[Michael Ventnor]

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) :-)

Comment 7

[Javi Rueda :javirid]

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.

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/2ff6999222c2