57621 - Links in Composer aren't drawn as links (underlined, blue)

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P3)

Description

[selmer (gone)]

10/20 17 MN6

My status report has a passel of links in a list structure.  Double click and
edit-as-new both bring up a compose window where the links are not colored or
underlined.  When I save the msg back to drafts, the links are colored and
underlined.  Could be a complication of 40340 if that was in the 10/20 17 build...

Comment 1

[selmer (gone)]

Also in 10/22 build.  Adding rtm keyword.

Comment 2

[lchiang]

This is a new regression.  You can just compose a new message and see this 
problem:

1.  new html message
2.  type some text
3.  select the text and click on the Link icon on the formatting toolbar
4.  type some URL into this link dialog
5.  click ok
6.  the linkified text in the compose window is not underlined.

This really needs to be fixed for rtm.

Composer has this same problem using 10/20 build!  Reassign to beppe

Comment 3

[Kathleen :Brade]

I see this on a Mac build
Another way that I can see this bug is by editing a page with links such as:
  * in a browser window, load http://www.mozilla.org/
  * from File menu, choose Edit Page
  * notice in Composer that the underlines are missing

Note: this is a cosmetic bug (in that the links are actually created or 
maintained in the html); the user just doesn't get the feedback that the link 
created is actually there

Marc--is this related to anything you've checked in recently?

Comment 4

[selmer (gone)]

Can someone please comment on whether this is a regression caused by checking in
for 40340???

Comment 5

[kinmoz]

I just verified that this was caused by attinasi's checkin to nsEditorShell.cpp 
rev 1.202.2.9:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&su
bdir=mozilla/editor/base&command=DIFF_FRAMESET&file=nsEditorShell.cpp&rev1=1.202
.2.8&rev2=1.202.2.9&root=/cvsroot

Comment 6

[kinmoz]

Adding myself to Cc list.

Comment 7

[lchiang]

Even though this is cosmetic, it's important feedback to the user because if I
ever need to change a link, I'd like to be able to thoroughly select the
previous linked text to change it.  Otherwise, I could end up with wrong links
or a word where the text has one link and a period or something has another link.

Comment 8

[Marc Attinasi]

The problem is pretty simple: composer is disabling the pref style sheet (the 
change mentioned to nsEditorShell.cpp), and that is disabling the rules that 
cause the link colors and the link underline.

There are a couple of ways to fix it: easiest is for the editor shell to add the 
link color rules to one if its CSS files (hard-coded link colors would be 
necessary, rather than the preferred link colors). The other way is to change 
the semantics of disabling the pref stylesheet so that it does not disable the 
link rules. I had planned to make the control of the pref style sheet a bit more 
flexible in the long term, but maybe it has to be now.

Personally, I prefer to simply add the link rules to one of the composer 
stylesheets (editorContent or editorOverride) - I'm testig this out. Does it 
seem reasonable to the composer team?

Comment 9

[Marc Attinasi]

Accepting. Obviously my regression. This should be very very easy to fix. I'm 
testing the composer CSS approach now.

Comment 10

[Marc Attinasi]

Putting the following rules into editorOverride.css fixes this:

:link, :link:active, :visited, :visited:active {
 text-decoration: underline;
}
:link{
 color: blue;
}
:visited{
 color: purple;
}

This is just taking the rules that used to be in html.css but are now in the 
pref stylesheet (which Composer disables) and adding them directly to 
composer's stylesheet.

This is the easiest way to fix the regression. It will not, however, allow the 
preferred link colors to be shown in composer, but then it never could before 
either. The other approach is to modify the code in the PresShell so that the 
link color and link underline rules are not disabled along with the other 
preference rules. This is not hard, but it is more risky than the CSS change.

Comment 11

[Marc Attinasi]

Attachment: Patch to fix the link colors and link underline via CSS (easiest way)

Comment 12

[selmer (gone)]

Please get the reviews so the PDT can approve this!

Comment 13

[Simon Fraser [no longer active]]

don gave his r= on this, and sr=sfraser.

Please don't check this onto the trunk; the trunk should be fixed so that the 
underline links pref is obeyed in composer too.

Comment 14

[don]

PDT sez check this puppy into the branch!

Comment 15

[Marc Attinasi]

Thanks, Simon. And, I'd add, the link color pref should be honored as well as 
the underline pref. Really, the composer should only be disabling the 'override' 
preferences, the ones that override the color and fonts specified on the page.

Checking in to trunk. Thanks.

Comment 16

[Marc Attinasi]

... er, branch - checked into branch, not trunk. BRANCH BRANCH BRANCH!

Leaving open for now, to remind me that this needs to be addressed on the trunk 
post-RTM.

Comment 17

[Marc Attinasi]

Actually, closing and opening a new bug for reminder. (Too much cold medicine 
tonight...)

Comment 18

[Marc Attinasi]

See bug 57757 for follow up / reminder.

Comment 19

[sujay]

verified in 10/24 build.