Last modified: 2010-01-10 16:40:29 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T23053, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 21053 - Broken diff header layout for multi diffs with no changes
Broken diff header layout for multi diffs with no changes
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
History/Diffs (Other open bugs)
1.16.x
All All
: Normal trivial with 1 vote (vote)
: ---
Assigned To: Robin Krahl
http://de.wikipedia.org/w/index.php?t...
: patch, patch-need-review
: 13875 21066 21433 (view as bug list)
Depends on:
Blocks: 18538 20601
  Show dependency treegraph
 
Reported: 2009-10-08 10:41 UTC by Robin Krahl
Modified: 2010-01-10 16:40 UTC (History)
10 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
sets width of the cols to 50% (254 bytes, patch)
2009-10-08 11:25 UTC, Robin Krahl
Details
Change colspan (1.26 KB, patch)
2009-10-08 12:56 UTC, Robin Krahl
Details

Description Robin Krahl 2009-10-08 10:41:10 UTC
The FlaggedRevs extension display two diffs in one table row instead of two rows if you’re looking at the diff between two version and there is no difference (e. g. if you reverted some vandalism).
Comment 2 Robin Krahl 2009-10-08 11:25:50 UTC
Created attachment 6649 [details]
sets width of the cols to 50%

The problem is that there are two tables: the actual diff table and the FlaggedRev’s table above. The width of the FR table is 100% and the width of their columns is 50%. The width of the diff table is fixed to (100%), too, but the width of their columns is not fixed. So if the columns have a different width, this bug occurs.

So in this proposed patch, I added width:50%; to the columns.
Comment 3 P.Copp 2009-10-08 11:32:26 UTC
You can easily reproduce this on en.wikipedia, (e.g http://en.wikipedia.org/w/index.php?title=Rumson,_New_Jersey&action=historysubmit&diff=317019545&oldid=317003264) so I don't think this has something to do with FlaggedRevs.

My guess is that the colspan="4" attribute of the multi diff notice is causing this.
Comment 4 Robin Krahl 2009-10-08 11:53:29 UTC
Yes, you’re rigth. (In reply to comment #3)
> You can easily reproduce this on en.wikipedia, (e.g
> http://en.wikipedia.org/w/index.php?title=Rumson,_New_Jersey&action=historysubmit&diff=317019545&oldid=317003264)
> so I don't think this has something to do with FlaggedRevs.
> 
> My guess is that the colspan="4" attribute of the multi diff notice is causing
> this.
> 

It was just my first thought that the bug may lie in the FlaggedRevs extension, but this is wrong, as you say. Nevertheless, my proposed patch fixes (or should fix) your example, too.
Comment 5 P.Copp 2009-10-08 12:16:09 UTC
(In reply to comment #4)
> 
> It was just my first thought that the bug may lie in the FlaggedRevs extension,
> but this is wrong, as you say. Nevertheless, my proposed patch fixes (or should
> fix) your example, too.
> 

Honestly, I think it's better to fix the layout than trying to override it with some css styles. The multi diff notice has a colspan of 4 whereas the rest of the table has only two columns.
Comment 6 Robin Krahl 2009-10-08 12:23:12 UTC
Ok. Would it bee all right to show the four <col>s everytime and add a colspan="2" to the header <td>s? This would fix the problem, too.
Comment 7 P.Copp 2009-10-08 12:33:15 UTC
(In reply to comment #6)
> Ok. Would it bee all right to show the four <col>s everytime and add a
> colspan="2" to the header <td>s? This would fix the problem, too.
> 

Apparently it had been this way until r49688, where it was changed to fix bug 18538. So better change the colspan of the multi diff notice to 2 if no diff is shown.
Comment 8 Robin Krahl 2009-10-08 12:56:54 UTC
Created attachment 6651 [details]
Change colspan

The error was that in the mentioned revision no one paid attention to the colspan of the diff-multi thing. So I added a $colspanHeader variable which deals with the problem.
Comment 9 Raimond Spekking 2009-10-09 10:00:53 UTC
*** Bug 21066 has been marked as a duplicate of this bug. ***
Comment 10 RockMFR 2009-10-11 18:11:09 UTC
The cause of the visible output change was actually r56406. Prior to that, the "if( $diff )" check would always evaluate to true because the debug output would always be present. So, that whole check is totally fubar. It should really be checking to see if something like "<tr>" is present. Though, that is a bit of a hack. Is there a way to do this without changing the colspans at all?
Comment 11 Brad Jorsch 2009-10-12 16:56:43 UTC
Interesting, Brion reported bug 18538 fixed in r49688 using "current safari/mac" on 28 April 2009, even though the change in r49688 was ineffectual until r56406. I've also just confirmed that diff output from r56405 (which I confirm seems to not be showing any effect due to r49688) displays correctly in Safari/Mac 4.0.3. Maybe r49688 can just be reverted, if it's only old versions of Safari that have the problem? Has anyone tested a recent version of Chrome?
Comment 12 Alexandre Emsenhuber [IAlex] 2009-10-22 15:12:20 UTC
Fixed in r58011.
Comment 13 Alexandre Emsenhuber [IAlex] 2009-11-08 08:10:40 UTC
*** Bug 21433 has been marked as a duplicate of this bug. ***
Comment 14 P.Copp 2010-01-10 16:40:29 UTC
*** Bug 13875 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links