Closed Bug 600133 Opened 10 years ago Closed 10 years ago

Bug 538890 broke about:buildconfig unification


(Toolkit Graveyard :: Build Config, defect)

Not set


(Not tracked)



(Reporter: alqahira, Assigned: alqahira)




(Keywords: regression)


(1 file, 1 obsolete file)

Apparently no-one ever tested the patch for bug 538890; it breaks buildconfig unification, and as a result, an <hr> and ugly x86_64 buildconfig info is all that's shown.

I think I have a fix, but since you have to be on 10.6 to build a Universal Binary any more, I've pushed my patch to try.
Attached patch fix? (obsolete) — Splinter Review
:grr bugzilla:
These empty <p>s were present in the original file introduced in 2003:
They used to add some margin before the <h2> elements.

There's no script in this file, nor any hint in the cvs/hg history that there's a "buildconfig unification", let alone where that happens.

So, this happens here:
> Copy the second file's content beginning after its leading <h1> and <p>.
The empty <p>s are not even needed by fix-buildconfig.

Please just fix that to not look for the first <p>, but the first </h1> instead, i.e. the <h1>about:buildconfig</h1> line, and don't add the hacky and unneeded empty <p>s back.
Thanks for filing this, and figuring out the regression. I noticed this recently but didn't have time to figure out what regressed it.
Attached patch Actual fixSplinter Review
I figured out a way to hack this up locally to test, so this should be the complete fix.  Patch is currently going to try for sanity-check, since "fix based on local hack" doesn't inspire as much confidence as I'd like, and since try has caught other gotchas for me in this now-two-bug process. ;)

This patch does four things:

1) Updates fix-buildconfig to not depend on the <p> </p> lines.
2) Since I'm cleaning up after bug 538890 here already (and also had these changes in my patch for bug 600133), this fixes cls and biesi to match every single other instance in the tree of them as contributors.
3) Removes the blank line that also breaks fix-buildconfig
4) Adds a warning to the file so that future modifiers will remember to test changes in a Universal binary, so that hopefully we'll never find ourselves in this state ever again ;)

I'll request review once there are working bits hot off try.
Attachment #478980 - Attachment is obsolete: true
(In reply to comment #4)
> changes in my patch for bug 600133), this fixes cls and biesi to match every
Make that bug 456360.
Comment on attachment 479131 [details] [diff] [review]
Actual fix displays the correct about:buildconfig again :)

>    # Copy the second file's content beginning after its leading <h1> and <p>.
Already fixed that comment locally to remove the ' and <p>' part that no longer is correct post-bug 538890 having removed the empty <p>s.

Gavin promised me last night he'd take duties on this if I fixed it.
Attachment #479131 - Flags: review?(
Attachment #479131 - Flags: approval2.0?
Comment on attachment 479131 [details] [diff] [review]
Actual fix

I'd mention fix-buildconfig specifically in the comment, r=me with that.
Attachment #479131 - Flags: review?(
Attachment #479131 - Flags: review+
Attachment #479131 - Flags: approval2.0?
Attachment #479131 - Flags: approval2.0+ (with an improved comment mentioning fix-buildconfig explicitly)
Closed: 10 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.