Closed
Bug 130728
Opened 23 years ago
Closed 19 years ago
messageBody.css should use namespaces correctly
Categories
(SeaMonkey :: MailNews: Message Display, defect)
SeaMonkey
MailNews: Message Display
Tracking
(Not tracked)
VERIFIED
FIXED
seamonkey1.1alpha
People
(Reporter: dbaron, Assigned: sgautherie)
References
Details
(Keywords: fixed-seamonkey1.1a, perf, verified1.8.1, Whiteboard: [verified-seamonkey1.1a])
Attachments
(7 obsolete files)
messageBody.css should use namespaces correctly -- it's current use has
performance problems. I think this was done because someone wanted to use
"html|.class" instead of the correct "html|*.class".
Note that the one existing selector that does have a namespace, the html|html
rule that gives a background color, should probably be **removed** to avoid
regressing bug 69436.
There's a patch for this already, within attachment 62713 [details] [diff] [review] on bug 91662.
Reporter | ||
Comment 1•23 years ago
|
||
Reporter | ||
Updated•23 years ago
|
Severity: normal → trivial
Status: NEW → ASSIGNED
Priority: -- → P5
Target Milestone: --- → Future
Comment 2•23 years ago
|
||
David, see my comments in bug 69436 about the background color. Let's discuss
this there.
I also already reviewed this patch in the other bug and had several comments,
which you seem to ignore here. Please see
<http://bugzilla.mozilla.org/show_bug.cgi?id=91662#c58> and following.
You say there:
> Using the default value rather than 'inherit' would make much more sense if
> that's what you want.
No, that's explicitly *not* what I want. For example, blockquote has a left
margin (and a bar with type=cite) by default, which in some cases is not wanted
(e.g. when just the original ">"s are shown).
Please make sure that everything still works exactly the same way as before
(modulo the decision in bug 69436), in all cases. See the *quote* prefs and
related prefs in default/pref/mailnews.js. Also try both format=flowed and
normal plaintext.
Reporter | ||
Comment 3•23 years ago
|
||
I'm not planning to fix this anytime soon. And anyway, 'inherit' still doesn't
make any sense. 'inherit' means inheritance from the parent content node. What
if that's an H1? Or a P? Or a DIV? You get different results, and that's not
what you want.
Comment 4•23 years ago
|
||
A blockquote within an h1???
What I get is that the element does not change anything. And that's what I want.
Reporter | ||
Comment 5•23 years ago
|
||
I think what you mean by that is that you want it to have 0 margin, padding, and
border. These are the initial values for the property. This is conceptually
very different from 'inherit' and the implementation is quite different (and
more expensive in memory use and time). (Note that CSS3 will probably have
special keyword 'initial' in addition to 'inherit' to represent this in general.)
Comment 6•23 years ago
|
||
> I think what you mean by that is that you want it to have 0 margin, padding, and
> border.
No. If the default is a blue bar, I don't want to set it to a blue bar. I want
to use the same bar color that the wrapping blockquote uses, which is ultimately
sat by a pref (generating HTML).
For these elements, I don't want them to have any effect on style. That's what
inherit does and initial wouldn't.
Reporter | ||
Comment 7•23 years ago
|
||
No, 'inherit' says that if the parent has a blue border, you want *two* blue
borders. That's definitely an effect on style.
Comment 8•23 years ago
|
||
Yes, in that case I wanted to have the one bar for each blockquote. Probably a
bad example.
You commented on the margin/border rules for blockquote/pre only. In that case,
in the outer blockquote has been sat to a margin: 1em, you probably want the
nested ones to have the same margin, not?
Note: I fiddled quite a lot with the rules when I wrote them, and they now work
in all cases I know of, even with user stylesheets.
If you have an concrete alternative proposal, test it with all the edge cases
and if the result is the same or better, feel free to submit a patch and we can
discuss about it.
Reporter | ||
Comment 9•23 years ago
|
||
All my patch has is a comment asking why. I think any use of inherit needs to
be explained clearly, and you certainly have the opportunity to do that after
this patch goes in, unless you want to give the explanations now.
Comment 10•23 years ago
|
||
> All my patch has is a comment asking why.
If you want to ask something, you know my email address. "XXX" means "fix me",
while I don't think that there's anything to fix. Please remove the comments.
> unless you want to give the explanations now.
Didn't I?
If it's just the margin/border widths of the pre/blockquotes, I have no strong
objections, if you set them to 0, *if* that breaks nothing.
You didn't yet say, why you don't use html as the default namespace. The message
body is mostly or even completely HTML (ordinary Transitional HTML, not XHTML, BTW).
Comment 11•22 years ago
|
||
Is this bug also about this css error, or should I file a new bug for that?
CSS Error
(jar:resource:///chrome/classic.jar!/skin/classic/messenger/messageBody.css
:52.6): Unknown namespace prefix 'html'. Selector expected. Ruleset ignored
due to bad selector.
Reporter | ||
Comment 12•22 years ago
|
||
Yes, the patch in this bug fixes that CSS error.
Comment 13•21 years ago
|
||
I thought it would be helpful to separate the invalid html namespace issue from
the other questions related to inherited etc.
Reporter | ||
Comment 14•21 years ago
|
||
Comment on attachment 139082 [details] [diff] [review]
(Av1b-MAS) version of patch addressing only the namespace issues
r=dbaron, although there are probably more copies of this file now
Attachment #139082 -
Flags: review+
Attachment #139082 -
Flags: superreview?(neil.parkwaycc.co.uk)
Comment 15•21 years ago
|
||
Comment on attachment 139082 [details] [diff] [review]
(Av1b-MAS) version of patch addressing only the namespace issues
I've trawled through the code, and found that this stylesheet is referenced
twice, once obviously in the HTML message display, and once in
nsMimeXmlEmitter. However as this is only used when mHeadersOnly is true, the
output is promptly discarded by QuotingOutputStreamListener::OnDataAvailable().
Therefore I'd want to see the default namespace should set to HTML instead,
removing XUL styles.
Attachment #139082 -
Flags: superreview?(neil.parkwaycc.co.uk) → superreview-
Comment 16•21 years ago
|
||
This address the review comments earlier (makes html default namespace, xul is
the other one).
Attachment #139082 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #159996 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #159996 -
Flags: review?(dbaron)
Comment 17•21 years ago
|
||
Comment on attachment 159996 [details] [diff] [review]
(Bv1-MAS) Patch to address review comments
I'll have to check when I get back, but I think some of those classes are used
when printing the message header table.
Comment 18•21 years ago
|
||
Comment on attachment 159996 [details] [diff] [review]
(Bv1-MAS) Patch to address review comments
window isn't used at all; mailattachcount, header and headerdisplayname are not
XUL, only "used" by the XML emitter while .header-partN are all HTML not XUL.
Attachment #159996 -
Flags: superreview?(neil.parkwaycc.co.uk) → superreview-
Comment 19•21 years ago
|
||
>(From update of attachment 159996 [details] [diff] [review])
>window isn't used at all; mailattachcount, header and headerdisplayname are not
>XUL, only "used" by the XML emitter while .header-partN are all HTML not XUL.
had no clue I should remove window...
mailattachcount, header, and headerdisplayname were not touched by original
patch, so I assumed they were xul (since there are no other namespaces).
classes I can fix, can you please advise me on what to do with |window| and with
the other entities?
Assignee | ||
Comment 20•21 years ago
|
||
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a4) Gecko/20040927] (release) (W98SE)
(line changed)
{{
Error: Unknown namespace prefix 'html'. Selector expected. Ruleset ignored due
to bad selector.
Source File: chrome://messenger/skin/messageBody.css
Line: 54
}}
This error is now reported in the JS.C. [new feature];
I hope it can be fixed before v1.8a5.
Reporter | ||
Updated•21 years ago
|
Attachment #159996 -
Flags: review?(dbaron)
Updated•20 years ago
|
Product: Browser → Seamonkey
Assignee | ||
Updated•20 years ago
|
Flags: blocking1.8a6?
Updated•20 years ago
|
Flags: blocking1.8a6? → blocking1.8a6-
Assignee | ||
Comment 21•20 years ago
|
||
Neil:
Could you answer the question(s) in comment 19 ?
(I guess you missed that comment since you were not added yet in the CC list...)
Flags: blocking1.8b?
Assignee | ||
Comment 22•20 years ago
|
||
Comment on attachment 159996 [details] [diff] [review]
(Bv1-MAS) Patch to address review comments
>Index: ./themes/classic/messenger/messageBody.css
>===================================================================
>
>+/* set default namespace to HTML */
>+@namespace xul url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
>+@namespace url("http://www.w3.org/1999/xhtml");
>
>
>Index: ./themes/modern/messenger/messageBody.css
>===================================================================
>
>+/* set default namespace to HTML */
> @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
> @namespace html url("http://www.w3.org/1999/xhtml");
I guess the 2 modern lines should be changed like the classic ones...
Comment 23•20 years ago
|
||
Callek, please carefully test in all the combination of cases (see end of
comment 2). This is the main work to be done in this bug. Comment 22 makes me
wonder, if this is tested at all. I reserve the right to demand a backout or
fast fix, should anybody find regressions. This bug sounds innocent, but has the
potential for hard-to-find regressions. That said, I'd really like this bug to
be fixed (was thinking of doing it myself).
Comment 24•20 years ago
|
||
(In reply to comment #19)
>classes I can fix, can you please advise me on what to do with |window| and
>with the other entities?
Just removing those four rogue tags (and html|html) should suffice, I think.
Comment 25•20 years ago
|
||
Ben or Serge, feel free to finish the patch for this bug (or someone else) I
wont have time for a while, and I imagine that at least some people want this
fixed ;-)
No need to credit me by the way ;-)
Assignee | ||
Comment 26•20 years ago
|
||
(In reply to comment #23) [Ben Bucksch]
> That said, I'd really like this bug to
> be fixed (was thinking of doing it myself).
(In reply to comment #25) [Justin Wood]
> Ben or Serge, feel free to finish the patch for this bug (or someone else) I
> wont have time for a while
I could update the patch, but my guess is that fully testing it is beyond my skill.
Ben, would you be ready to assign this bug to yourself and deal with it ?
Severity: trivial → blocker
Assignee | ||
Updated•20 years ago
|
Severity: blocker → normal
Comment 27•20 years ago
|
||
> Ben, would you be ready to ... deal with it ?
Probably not at the moment. Not sure.
Assignee | ||
Updated•20 years ago
|
Attachment #73978 -
Attachment description: patch → (Av1-MAS) patch
Attachment #73978 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #139082 -
Attachment description: version of patch addressing only the namespace issues → (Av1b-MAS) version of patch addressing only the namespace issues
Assignee | ||
Updated•20 years ago
|
Attachment #159996 -
Attachment description: Patch to address review comments → (Bv1-MAS) Patch to address review comments
Attachment #159996 -
Attachment is obsolete: true
Assignee | ||
Comment 28•20 years ago
|
||
Bv1, with comment 24 suggestion(s).
Is |@namespace url("http://www.w3.org/1999/xhtml");| wanted in
<messageBody.css> ?
If yes, should that same line be added to <smileys.css> (in a separate patch) ?
Implicitly, I removed |div.headerdisplayname| as a "fifth rogue tag"...
I noticed that these 2 theme files are (now) identical, but I guess that's
wanted as is.
As I warned, this patch is untested: helpwanted !
Attachment #171384 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #171384 -
Flags: review?(dbaron)
Comment 29•20 years ago
|
||
Comment on attachment 171384 [details] [diff] [review]
(Bv2-MAS) <messageBody.css>
(In reply to comment #28)
>Is |@namespace url("http://www.w3.org/1999/xhtml");| wanted in
><messageBody.css> ?
That's the whole point of this bug.
>If yes, should that same line be added to <smileys.css> (in a separate patch)?
I'd accept them both in a single patch.
>Implicitly, I removed |div.headerdisplayname| as a "fifth rogue tag"...
That's not how CSS works. div is the tag, and headerdisplayname is actually a
class name. The generated HTML looks like <div class="headerdisplayname">.
>I noticed that these 2 theme files are (now) identical, but I guess that's
>wanted as is.
That sounds about right to me for now, though at some point scott/bienvenu
might want to move them to a single file in content.
Attachment #171384 -
Flags: superreview?(neil.parkwaycc.co.uk) → superreview-
Assignee | ||
Updated•20 years ago
|
Attachment #171384 -
Attachment is obsolete: true
Attachment #171384 -
Flags: review?(dbaron)
Assignee | ||
Comment 30•20 years ago
|
||
Bv2-MAS, with comment 29 suggestion(s).
(These 2 theme <smileys.css> are identical too.)
As I warned, this patch is untested...
Assignee: dbaron → gautheri
Attachment #171462 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #171462 -
Flags: review?(dbaron)
Reporter | ||
Comment 31•20 years ago
|
||
Comment on attachment 171462 [details] [diff] [review]
(Bv3-MAS) <messageBody.css> ++
[Checked in: Comment 57]
clearing review request since previous reviewers have requested specific
testing
Attachment #171462 -
Flags: review?(dbaron)
Assignee | ||
Updated•20 years ago
|
Priority: P5 → --
Whiteboard: [helpwanted: looking for a patch tester...]
Target Milestone: Future → mozilla1.8beta
Reporter | ||
Updated•20 years ago
|
Flags: blocking1.8b? → blocking1.8b-
Assignee | ||
Updated•20 years ago
|
Flags: blocking1.8b2?
Comment 32•20 years ago
|
||
This is so not a blocker.
Updated•20 years ago
|
Flags: blocking-seamonkey1.0a?
![]() |
||
Comment 33•20 years ago
|
||
We (or better MoFo) have been shipping a few releases with that without
problems, so we can also ship an alpha with it. That doesn't mean it can't go
in, just that we won't hold alpha for it :)
Flags: blocking-seamonkey1.0a? → blocking-seamonkey1.0a-
Comment 34•20 years ago
|
||
(In reply to comment #31)
> clearing review request since previous reviewers have requested specific
> testing
dbaron, I'm willing to test this if it's still needed.
Assignee | ||
Comment 35•20 years ago
|
||
(In reply to comment #34)
> dbaron, I'm willing to test this if it's still needed.
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050705 SeaMonkey/1.0a]
(nightly) (W98SE)
Bug still there ... your help would be welcomed !
Comment 36•20 years ago
|
||
Thunderbird version 1.5 Beta 2 (20051006) (WinXP)
Error: Unknown namespace prefix 'html'. Ruleset ignored due to bad selector.
Source File: chrome://messenger/skin/messageBody.css
Line: 55
Assignee | ||
Comment 37•19 years ago
|
||
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8) Gecko/20051215 SeaMonkey/1.0b] (nightly) (W98SE)
(Someday maybe.)
Comment 38•19 years ago
|
||
Requesting blocking-seamonkey1.0. A prior version of this patch has r=dbaron, and I believe it's SeaMonkey only. It does not affect UI, and predates the SeaMonkey project. At best, the goal is to silence an error.
Flags: blocking-seamonkey1.0?
This hardly seems severe enough to block for. However, it also sounds low-risk enough that a patch may be acceptable post-beta.
Flags: blocking-seamonkey1.0? → blocking-seamonkey1.0-
Assignee | ||
Comment 40•19 years ago
|
||
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051230 SeaMonkey/1.5a] (nightly) (W98SE)
The error was reduced to a warning
{{
Warning: Unknown namespace prefix 'html'. Ruleset ignored due to bad selector.
Source File: chrome://messenger/skin/messageBody.css
Line: 54
}}
by a checkin in the last week.
Comment 41•19 years ago
|
||
*** Bug 322653 has been marked as a duplicate of this bug. ***
Comment 42•19 years ago
|
||
(In reply to comment #40)
And I get this warning all the time. can't someone change that
html|html {
line?
Comment 43•19 years ago
|
||
(In reply to comment #30)
> As I warned, this patch is untested...
So far no problems with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060127 SeaMonkey/1.0, patch applied. The error/warning mentioned above doesn't appear any more, can't find any regressions from the patch.
Comment 44•19 years ago
|
||
Serge: It seems people report the patch is working - how about a review?
Assignee | ||
Comment 45•19 years ago
|
||
(In reply to comment #44)
> Serge: It seems people report the patch is working - how about a review?
Comment 43 doesn't seem enough to me, compared to what comment 23 asks for.
Ben, Alex:
Would one of you have time now to do the extended test part ?
Updated•19 years ago
|
Attachment #171462 -
Flags: superreview?(neil)
Attachment #171462 -
Flags: superreview+
Attachment #171462 -
Flags: branch-1.8.1?(mscott)
Assignee | ||
Comment 46•19 years ago
|
||
Comment on attachment 171462 [details] [diff] [review]
(Bv3-MAS) <messageBody.css> ++
[Checked in: Comment 57]
From a Neil's email:
{{
I did a quick test on it, e.g. smileys still work.
}}
Attachment #171462 -
Flags: review?(dbaron)
Reporter | ||
Comment 47•19 years ago
|
||
What's all the removed stuff? Why is it not needed anymore?
Comment 48•19 years ago
|
||
> What's all the removed stuff? Why is it not needed anymore?
See comment #18.
Reporter | ||
Comment 49•19 years ago
|
||
(In reply to comment #18)
> mailattachcount, header and headerdisplayname are not
> XUL, only "used" by the XML emitter
Does this mean there's no need to style them?
Comment 50•19 years ago
|
||
(In reply to comment #47)
> What's all the removed stuff? Why is it not needed anymore?
FWIW, using Seamonkey 1.0, I loaded a message window with a forwarded message (invoking the messageBody.css file) into the DOM Inspector and checked the 'window' element and the 'html' element, neither which load any rules from
messageBody.css. I also loaded a .EML file into the browser and checked the 'window' -- same thing; but for some reason, I couldn't drill down to the display of the message; the tabbrowser node (id = content) showed no subnodes
in DOMi. At any rate, the sorts of subtle regressions Ben was worrying about seem unlikely as the rules for those elements don't seem to be applied
anywhere.
Somewhat OT, I note that the rule in messageBody.css for
|div.headerdisplayname| setting |white-space:pre;|
is superfluous: that's being applied only to the header name in the table of headers, and the header name is always one word (altho, sometimes with
hyphens). There is no whitespace to be controlled; and since that .headername <div> has |display:inline;| the 'white-space' rule doesn't apply anyway.
(Why a <div> instead of a <span>?)
Reporter | ||
Comment 51•19 years ago
|
||
Comment on attachment 171462 [details] [diff] [review]
(Bv3-MAS) <messageBody.css> ++
[Checked in: Comment 57]
OK, it's ok with me but I'd to double-check with mscott that those rules aren't needed.
Attachment #171462 -
Flags: review?(mscott)
Attachment #171462 -
Flags: review?(dbaron)
Attachment #171462 -
Flags: review+
Assignee | ||
Comment 52•19 years ago
|
||
(In reply to comment #50)
> Somewhat OT, I note that the rule in messageBody.css for
> |div.headerdisplayname| setting |white-space:pre;|
> is superfluous
David, Neil, Scott:
Then, should I remove that |white-space:pre;| line too ?
Comment 53•19 years ago
|
||
(In reply to comment #50)
>I also loaded a .EML file into the browser and checked the 'window'
Easier is to press Ctrl+Shift+I which loads the current tab into a new DOMI window. Otherwise you have to remember to view anonymous content.
(In reply to comment #52)
>should I remove that |white-space:pre;| line too ?
You never know, someone might implement hyphenation...
Comment 54•19 years ago
|
||
Are we really really sure these rules aren't used anymore? We still use the classnames in the various mime emitters including the base emitter which uses headerdisplayname:
http://lxr.mozilla.org/mozilla/source/mailnews/mime/emitters/src/nsMimeBaseEmitter.cpp#737
Comment 55•19 years ago
|
||
(In reply to comment #54)
>the base emitter [snip] uses headerdisplayname:
We actually declare it twice, so we're removing the unused version:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/themes/classic/messenger/messageBody.css&rev=1.13&mark=69,83#68
Comment 56•19 years ago
|
||
Comment on attachment 171462 [details] [diff] [review]
(Bv3-MAS) <messageBody.css> ++
[Checked in: Comment 57]
thanks for the explanation Neil.
Attachment #171462 -
Flags: review?(mscott)
Attachment #171462 -
Flags: review+
Attachment #171462 -
Flags: approval-branch-1.8.1?(mscott)
Attachment #171462 -
Flags: approval-branch-1.8.1+
Comment 57•19 years ago
|
||
Fix checked in to the trunk and branch.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: fixed-seamonkey1.1a,
fixed1.8.1
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Attachment #171462 -
Attachment description: (Bv3-MAS) <messageBody.css> ++ → (Bv3-MAS) <messageBody.css> ++
[Checked in: Comment 57]
Attachment #171462 -
Attachment is obsolete: true
Assignee | ||
Comment 58•19 years ago
|
||
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20060315 SeaMonkey/1.5a] (nightly) (W98SE)
V.Fixed.
Status: RESOLVED → VERIFIED
Whiteboard: [helpwanted: looking for a patch tester...]
Target Milestone: mozilla1.8beta1 → seamonkey1.1alpha
Assignee | ||
Comment 59•19 years ago
|
||
(In reply to comment #36)
> Thunderbird version 1.5 Beta 2 (20051006) (WinXP)
>
> Error: Unknown namespace prefix 'html'. Ruleset ignored due to bad selector.
> Source File: chrome://messenger/skin/messageBody.css
> Line: 55
I had missed this comment :-(
[Mozilla Thunderbird, version 3 alpha 1 (20060403)] (nightly) (W98SE)
This bug is still present in TB :-/
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 60•19 years ago
|
||
Attachment #217344 -
Flags: review?(bryner)
Assignee | ||
Comment 61•19 years ago
|
||
I left alone 'mailattachcount' in both files, as it has a color in pinstripe...
Untested patch; comment welcomed.
Attachment #217348 -
Flags: review?
Assignee | ||
Updated•19 years ago
|
Attachment #217348 -
Flags: review? → review?(mscott)
Comment 62•19 years ago
|
||
I'll try out the TB patch.
Assignee | ||
Comment 63•19 years ago
|
||
Comment on attachment 171462 [details] [diff] [review]
(Bv3-MAS) <messageBody.css> ++
[Checked in: Comment 57]
(In reply to comment #39)
> This hardly seems severe enough to block for. However, it also sounds low-risk
> enough that a patch may be acceptable post-beta.
Bug is still present in
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.0.1) Gecko/20060130 SeaMonkey/1.0] (release) (W98SE)
'approval‑seamonkey1.0.2=?':
Trivial U.I. code cleanup, no risk.
Attachment #171462 -
Flags: approval-seamonkey1.0.2?
Updated•19 years ago
|
Attachment #217348 -
Flags: review?(mscott) → review+
Comment 64•19 years ago
|
||
I'll land this -I've been running with it.
Comment 65•19 years ago
|
||
TB fix checked in, thx very much, Serge.
Assignee | ||
Comment 66•19 years ago
|
||
Comment on attachment 217348 [details] [diff] [review]
(Dv1) </mail> ThunderBird port
[Checkin: Comment 65]
'approval-branch-1.8.1=?': (ThunderBird only)
Simple U.I. code cleanup, no risk.
(You previously a+ the SM part.)
Attachment #217348 -
Attachment description: (Dv1) </mail> ThunderBird port → (Dv1) </mail> ThunderBird port
[Checkin: Comment 65]
Attachment #217348 -
Flags: approval-branch-1.8.1?(mscott)
Assignee | ||
Comment 67•19 years ago
|
||
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8) Gecko/20060414 SeaMonkey/1.1a] (nightly) (W98SE)
V.Fixed for SMv1.1a.
Whiteboard: ['fixed1.8.1' applies to Bv2-MAS patch only] [verified-seamonkey1.1a]
Updated•19 years ago
|
Attachment #217348 -
Flags: approval-branch-1.8.1?(mscott) → approval-branch-1.8.1+
Assignee | ||
Comment 68•19 years ago
|
||
Comment on attachment 217348 [details] [diff] [review]
(Dv1) </mail> ThunderBird port
[Checkin: Comment 65]
'approval1.8.0.3=?': (ThunderBird only)
Simple U.I. code cleanup, no risk.
Attachment #217348 -
Flags: approval1.8.0.3?
Reporter | ||
Comment 69•19 years ago
|
||
This doesn't seem anywhere remotely close to the criteria for security+stability releases. Please stop nominating bugs that don't fit the criteria.
http://developer.mozilla.org/devnews/index.php/2005/12/16/whats-next/
Assignee | ||
Comment 70•19 years ago
|
||
(In reply to comment #69)
> This doesn't seem anywhere remotely close to the criteria for
> security+stability releases. Please stop nominating bugs that don't fit the
> criteria.
>
> http://developer.mozilla.org/devnews/index.php/2005/12/16/whats-next/
It may not be close enough (meaning approval will be minused), but I don't see it as far as you say: let me explain.
1) Security: it's clearly not.
2) "reliability": it may not be a "major performance issues", but you did add the 'perf' keyword. (see comment 0)
3) Then:
"Has been on the trunk with no regressions reported in 2 weeks": yes, first checkin happened a month ago.
"Must have a clear explanation in the bug of":
"Summary of the changes in the fix": cleanup, change namespaces between html and xul.
"Risk assessment": none/low, already commented on between you and others.
"A reproducible testcase": displaying an email.
"l10n impact": none.
"Does not change any public API": (N/A).
"Does not change website compatibility or rendering": (N/A).
And it was said:
"This hardly seems severe enough to block for. However, it also sounds low-risk
enough that a patch may be acceptable post-beta."
This is what led me to ask for Gv1.8.0 branch too.
Reporter | ||
Comment 71•19 years ago
|
||
(In reply to comment #70)
> 3) Then:
Not relevant, since that part only applies if something from the first list applies.
But how about not trying to twist the text to fit your intent and instead applying some common sense?
Assignee | ||
Updated•19 years ago
|
Attachment #171462 -
Flags: approval-seamonkey1.0.2?
Assignee | ||
Updated•19 years ago
|
Attachment #217348 -
Flags: approval1.8.0.3?
Assignee | ||
Comment 72•19 years ago
|
||
(In reply to comment #71)
> But how about not trying to twist the text to fit your intent and instead
> applying some common sense?
My intent was to improve stable releases too, but I stand corrected.
*****
(In reply to comment #65)
> TB fix checked in, thx very much, Serge.
Thanks to you (and others) too !
Did anyone have a chance to check whether I was right when "doing":
"I left alone 'mailattachcount' in both files, as it has a color in pinstripe...",
since it was removed in the xpfe patch( see comment 18) ?
*****
(In reply to comment #60)
> Created an attachment (id=217344) [edit]
> (Cv1) </Toolkit> port
Now, I read what scott wrote in bug 321389 comment 3, which probably applies to my patch too:
"Im pretty sure this patch isn't going to do anything. Thunderbid no longer
builds tookit\themes\qute, instead we use winstripe now. I haven't checked in
the code to cvs remove the files yet."
Scott, what should we do about this ?
Assignee | ||
Comment 73•19 years ago
|
||
Attachment #217344 -
Flags: review?(mscott)
Assignee | ||
Comment 74•19 years ago
|
||
Comment on attachment 217348 [details] [diff] [review]
(Dv1) </mail> ThunderBird port
[Checkin: Comment 65]
Checkin: { 2006-04-23 02:19 bugzilla%standard8.demon.co.uk MOZILLA_1_8_BRANCH }
Attachment #217348 -
Attachment is obsolete: true
Assignee | ||
Updated•19 years ago
|
Whiteboard: ['fixed1.8.1' applies to Bv2-MAS patch only] [verified-seamonkey1.1a] → [verified-seamonkey1.1a]
Assignee | ||
Comment 75•19 years ago
|
||
[Mozilla Thunderbird, version 2 alpha 1 (20060425)] (nightly) (W98SE)
V.Fixed for TBv2.0a1
Keywords: fixed1.8.1 → verified1.8.1
Assignee | ||
Comment 76•19 years ago
|
||
Comment on attachment 217344 [details] [diff] [review]
(Cv1) </Toolkit> port
(This file does not exist anymore.)
Attachment #217344 -
Attachment is obsolete: true
Attachment #217344 -
Flags: review?(mscott)
Attachment #217344 -
Flags: review?(bryner)
Assignee | ||
Comment 77•19 years ago
|
||
Whether I was right to keep the 'mailattachcount' rules in the <mail/> patch was not answered,
but I'm resolving this bug anyway.
Status: REOPENED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•