Last modified: 2012-04-05 19:50:08 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 T29375, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 27375 - Whitespace changes should be identifiable in CR diffs
Whitespace changes should be identifiable in CR diffs
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
CodeReview (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: MW 1.20 version
Assigned To: Krinkle
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-13 02:57 UTC by p858snake
Modified: 2012-04-05 19:50 UTC (History)
4 users (show)

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


Attachments

Description p858snake 2011-02-13 02:57:40 UTC
Krinkle added this in common.css, It's probably nice and usable enough to do in the core of CR:

.mw-codereview-diff ins {
    background: #DDFFDD;
}
.mw-codereview-diff del {
    background: #FFDDDD;
}
Comment 1 Sam Reed (reedy) 2011-02-13 16:39:05 UTC
This doesn't seem to make any difference on my brower, but on my phone it seems to do...
Comment 2 Sam Reed (reedy) 2011-02-13 18:28:30 UTC
Oh, seen it locally. Duh, diffs get cached

r82064
Comment 3 Sam Reed (reedy) 2011-03-26 19:36:56 UTC
Reverted r82064 in r84818, reopening this, closing bug 27416
Comment 4 Krinkle 2011-06-25 23:39:49 UTC
This can be re-done. 

With bug 29307 fixed, bug 27416 will not occur anymore when background styling is added.
I've added it to MediaWiki.org earlier this month:

http://www.mediawiki.org/w/index.php?title=MediaWiki:Common.css&diff=prev&oldid=408365
Comment 5 Sam Reed (reedy) 2011-06-26 02:19:57 UTC
(In reply to comment #4)
> This can be re-done. 
> 
> With bug 29307 fixed, bug 27416 will not occur anymore when background styling
> is added.
> I've added it to MediaWiki.org earlier this month:
> 
> http://www.mediawiki.org/w/index.php?title=MediaWiki:Common.css&diff=prev&oldid=408365

So it was you :P

I did notice it seemed to change back again, but wasn't overly sure why

Is it those bits just need re-adding?
Comment 6 Krinkle 2011-06-26 02:24:34 UTC
Not the exact bits (colors are slightly changed) but the bits in the linked diff on mediawiki.org [1].

Also this depends on r89697 which fixed bug 29307.
Comment 7 Krinkle 2012-02-26 20:42:37 UTC
The latest changes in the extension (which, among other things, changed the diff presentation into a *cough* table), have made it impossible to highlight trailing whitespace because the parent elements of the text are no longer inline elements and styling their background has no effect.
Comment 8 Chad H. 2012-02-26 23:56:38 UTC
(In reply to comment #7)
> The latest changes in the extension (which, among other things, changed the
> diff presentation into a *cough* table), have made it impossible to highlight
> trailing whitespace because the parent elements of the text are no longer
> inline elements and styling their background has no effect.

The latest changes make the diffs look like crap, IMHO.
Comment 9 Krinkle 2012-02-26 23:58:37 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > The latest changes in the extension (which, among other things, changed the
> > diff presentation into a *cough* table), have made it impossible to highlight
> > trailing whitespace because the parent elements of the text are no longer
> > inline elements and styling their background has no effect.
> 
> The latest changes make the diffs look like crap, IMHO.

I'll see if I can change the diffHighlighter to maintain a inline-level element wrapper around the diff text so that background colors can still be applied without highlighting the entire horizontal line but just the text width.

WIll probably change <td class="ins">..</td> to be <td><ins>..</ins></td>
Comment 10 Krinkle 2012-02-27 00:12:10 UTC
Done in r112458. After it gets reviewed and deployed we can test the styling on mw.org and then I'll commit them.
Comment 11 Antoine "hashar" Musso (WMF) 2012-02-27 20:24:37 UTC
In previous fix HTML entities where showing. Fixed by r112460

Both revisions merged to 1.19wmf1 with r112464 and r112459 have been applied on live site.
Comment 12 Krinkle 2012-02-27 20:38:55 UTC
Bug fixed in r112511.

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


Navigation
Links