using the build from 5/26 on win98 If I bring in a document or if I create a new document and I toggle through the modes, then in wyiswyg mode or html source mode I add additional elements, then toggle back to Show All Tags mode, some of the icons are missing.

Are you sure this isn't the bug where tags/content added in HTML Source mode are sometimes lost when switching to any of the other modes? Confirm this by using Debug | Output HTML after changing out of HTML Source and observe if all of the new tags/content was pasted correctly into the "normal" document.

no, none of the elements are lost, the text and element are still there, just the element icon does not display.

setting to m17, this is really going to confuse users, it looks like elements are being deleted.

Beth: Can you please supply a test file that reproduceably demonstrates the error?

sure, as soon as the commercial build is back up and running.

ah thanks Charley -- but, here is the steps I did to reproduce this: 1. open New Blank Page, stay in default Normal mode 2. add 4 lines of text, tag the first line as a heading. Highlight some text (not all) in the second line and make it bold element, highlight some more text in the second line and make it italic. Highlight the 4th line and mark it as a paragraph 3. select the Show All Tags mode. You should see the BODY element icon, the first line of text should have a heading element icon, the second line of text should have a bold and italic element icon, the fourth line shuold have a paragrapgh element icon 4. Toggle back to the Normal mode and then toggle back to the Show All Tags mode. All of the element icons except for the BODY element icon have disappeared. The text is still present, nothing has been deleted. 5. select HTML Source mode, you will notice that all of the elements are still there. 6. Toggle back to the Show All Tags mode, highlight some text on the third line and mark it as underlined 7. Toggle back to the Normal mode and back to the Show All Tags mode -- the underline element icon has now disappeared. 8. Toggle back to Normal mode -- notice everything renders as you marked it, nothing is deleted, it's just that in the Show All Tags mode, the element icon representations are not displayed.

We are having problems dealing with some tags, so the 2nd file removes those. Marc: We are now seeing problems displaying the background images for the tags. What I'm seeing today is that the first time I use "Show All Tags" mode, all of the "icons" (background images on the tags) display ok. Then switch back to normal, and then use "Show All Tags" again. All of the icons except "body" are missing after the 2nd try. After the first stylesheet load, we are storing the override style sheed as a member variable of the editor as a COM ptr, then reloading it each time "Show All Tags" is used again. Since you seem to understand the CSS/background image issues very well, you seem to be the best candidate to investigate this.

I'll look it over... Charles, instead of unloading and reloading the style sheet why don't you just enable/disable it? I recently added code to check if a stylesheet is enabled before using it in the cascade (somehow that bit was never checked before). It might be faster for you, especially if the stylesheet is not changing, you know, like, it's a static css file we distribute with the product.

Good suggestion! Does enabling/disabling work with override sheets? Can you please point me to the interface for enabling/disabling?

I found nsIStyleSheet::SetEnabled(). We store as nsICSSStyleSheet, so this will be easy. Thanks!

eMail from Charles Manske: Well, I tried using nsIStyleSheet::SetEnable and I still see the bug. I'm not sure if the existing code does this, bug: After the first use of "Show All Tags" mode, then 1st switch back to "Normal", the "body" icon is not removed (I think we saw that problem before, correct?) Then if you use "Show All Tags" again, the other icons don't redisplay, just as they don't now (using RemoveOverrideStyleSheet).

Marc I don't know if this is related or not, but on win98, when I toggle to HTML Source and back to either Normal or Show Tags, I lose all of the data that was within the body. I will open a bug for that.

Zoinks! that doesn't sound like a style problem, Beth. Please CC me on the new bug. I winder if there is a more fundamental problem here... The Show All Tags stuff worked, albeit for only a brief period, but I played with it at length when I fixed the background color problems. I don't really understand how the editor handles the document being edited, so I am having a hard time emulating what is going on in the editor in simpler testcases. Beth, what is the priority on this specific bug? Right now it is kind of low priority for me (not nsbeta2, not dogfood, not blocker) so I'm only looking at it in my spare cycles...

you're right, this should be bumped up, I will nominate for nsbeta2 -- this could really confuse users if they toggle, you get the impression that data is being removed.

I'm not sure if this is a separate issue, but I tried using nsIStyleSheet::SetEnable(PR_FALSE) instead of removing the style sheet to transition from "Show All Tags" mode to "Normal", as suggested by Marc, and when I do that the icons are removed from the page, but not the padding that is used to reserve space for the icon (which is a background image). I.e., disabling a style sheet that has "padding-left: 20px;" doesn't trigger redisplaying the page to remove the padding (act like "padding-left: 0px").

Charles, when a sheet is enabled or disabled the document associated with it is notified, which causes a bunch of other things to be notified, notably the PresShell which then does a reframe. If the StyleSheet does not have an associated document then none of this happens. I thought that was what I was seeing when you were doing the Load/Unload logic, the sheet had no document. Somewhere the style sheet needs to have the document set, by calling SetOwningDocument. Are you setting the document when you load the stylesheet?

Another possibility is that CSSStyleSheetImpl::SetEnabled could use the CSSStyleSheetImpl::GetOwningDocument method to get the document to notify rather than just using the mDocument data member... This would only help if the parent of the style sheet had a document but the style sheet does not, probably not this bug. It may be a good idea anyway, since it would take care of enabled child sheets (which will not work now).

We weren't setting the document when loading the sheet before, but I discovered that problem and added that to use SetEnable. So my tree has that fix, but that didn't seem to cure the buge. I sent that to you as part of the diff, which I'll attach the bug. Maybe I should try to get permission to check that in today. Would you prefer if I also checked in the code to use SetEnable() instead of the current load/unload code?

Charles, I think the Enable/Disable method is better, as well as easier to debug. Since neither methods work any better or worse I'd suggest using the Enable/Disable method. I'll try and apply the patch and see what happens. I'm assuming that you are running against tip layout code? Also, with the SetDocument call in there are you at least seeing the layout problem fixed?

Putting on [nsbeta2-] radar. Not critical to beta2. Putting on nsbeta3 keyword radar.

To answer Marc: No, setting the document didn't seem to fix anything. I disagree with the nsbeta2- assessment and would like reconsideration: We are loading/unloading, enabling/disabling style sheets to do the "Show All tags" mode. And as a result we are simply setting background images on tags when the sheet is loaded/enabled. The fact that this doesn't work is not just an isolated editor issue, but reflects something more fundamentally wrong with style sheets and/or background images. It seems those basic issues need to be addressed sooner than beta3. Note also that this is a regression -- this all worked a couple of weeks ago.

RickG comments that we should find out what the real problem is. Need to understand what the user impact of not fixing this for beta2.

Pierre -- can you help with this? Rick: It would be helpful if I could be allowed to checkin the code in the 6/08/00 patch so either Marc or Pierre will be working with the most-correct stylesheet usage. The impact on the user is that the "Show All Tags" editmode, which is a completely new, innovative way of editing is totally unusable, and we were hoping to get some good feedback about this in Beta2. As I argued above, it reveals some basic problem with loading/unloading, or enabling/disabling stylesheets that I feel is better addressed sooner than later.

It's a common tendency to believe that a problem which lies in a different module reveals a basic flaw with wide implications but in that particular case, I wouldn't use this argument to raise the priority of the bug. The real question on your side is: do we need Show All Tags for beta2? The PDT team didn't think so. You may argue the case with Beth but I don't think you can claim basic flaws in the style system to help your cause: - We enable/disable stylesheets when switching between quirks mode and standard mode and AFAIK there isn't any problem. - We used to load/unload style sheets in the Editor for the (now defunct) Apply Stylesheet command and there wasn't any problem either. - When reproducing the bug using beppe's comments from [2000-06-02 13:38], the icons are indeed missing but you can also notice that they are replaced by an empty space which corresponds to the size of the icon, meaning that the style wasn't completely lost: just the icon. Most importantly, if you quit the app and modify the stylesheet (EditorAllTags.css) to display backgrounds of different colors instead of icons, it works: the style is applied/removed and you can toggle between modes. If this is a style problem, it is limited to the 'background-image' property (and Marc may want to have a quick look because he worked in that area 1 or 2 months ago) otherwise it could be another tricky bug with image loading and the Cache.

There isn't any problem with the background-image property either. I did a quick test where I used the following line in EditorAllTags.css for the B and U tags: background-image: url(http://bugzilla.mozilla.org/ant.jpg); then I followed beppe's instructions to reproduce the bug and the image didn't disappear for B and U elements. This is not a style problem, removed the [NEED INFO] tag, reassigned to cmanske. It could be another image loading problem between ImageLib, Cache and History. Charles: a good test would be to replace the 'chrome:\\' urls with 'http:\\' urls of the same icons on a remote server then with 'file:\\' urls on your local machine.

Putting on [nsbeta2-] radar. We do not see evidence that this is serious enough to fix right now. Need a common user impact.

really? So, you're anew user using Composer and you type your little heart out and select Show All Tags to see what this HTML stuff is all about. ANd you say -- hey that's pretty cool. SO you toggle back to the Normal mode because you don't feel comfortable in the show all tags mode, and you enter more stuff - and want to sneak a peek at the new stuff in show all tags mose -- BUT WAIT: your stuff is gone - and you call that low impact? What am I not understanding?

Beth, I can't get the content to disappear, just the icons. Is it on Win98 only? Is the content really gone or does it come back after doing a resize or switching back to HTML Source?

Beth, I could reproduce the problem on Win98 (albeit running in VirtualPC). The easiest way for me to reproduce it is to click repeatedly on "Normal" while the document is still loading in the "HTML Source" mode. If I wait until the document is fully loaded in "HTML Source" mode, then click on "Normal", the problem doesn't happen. When the problem does happen, the content always comes back after either resizing the window or switching back to the previous mode. Both are fairly easy workarounds.

Independent of whether we identify and fix the image appearance problems for beta2, can I *please* checkin the simple patch attached above. All of you except Marc and I are using code that is not "correct" and this is a waste of time. As Marc has pointed out, we need to set the document when first loading the stylesheet, and then use SetEnabled(PR_FALSE or PR_TRUE) to disable and reenable the sheet instead of unloading are reloading it. With this code in, we can correctly evaluate where the problem really is. This is a very safe fix even if we don't make any further changes before beta2.

nsbeta2+ until 6/15

Assigning all nsbeta2+ bugs to M16.

Marc: The code to use SetEnabled() is checked in. We now have the additional problem that when switching from "Show All Tags" mode to "Normal" or "Preview", the extra padding-left used in the CSS for all the tags is not removed when SetEnabled(PR_FALSE) is used to turn off the EditorAllTags.css override sheet. Could you please investigate that problem?

I am also noticing the after switching back to "Normal" from "Show All Tags" mode, and then creating new content (e.g., click on one of the list toolbar buttons), the new content is shown with the icons that should only display in "Show All Tags" mode. Thus while Pierre is probably correct that the image disappearance is not a CSS problem, the latest issues I described are CSS-related. Marc doesn't have any other pressing bugs and would like to look at these new problems -- he was on vacation yesterday and we would like a couple more days to investigate.

Looking into this I have found a problem with Enable/Disable of the StyleSheet. Basically, we use the enable-bit on the sheet when we are building up the rule processors to see if that sheet's rules apply. However, when we disable or enable the sheet the rule processors are not rebuilt, so we are still using the old rules regardless of the enable-bit. I am testing a change that will rebuild the rule processors after a style sheet changes state... stay tuned.

Rebuilding the rule processors when a stylesheet's state changes definitely helps! With this change many of the problems of switching between ShowAllTags and Normal are eliminated. There are still problems with the images not showing up after the first time, however the layout is making room for them. Also, the BODY tag image is disappearing when you switch to normal mode now. The change is simple: added a new method on the nsIStyleSet interface and implemented in the StyleSetImpl: NS_IMETHODIMP StyleSetImpl::NotifyStyleSheetStateChanged(PRBool aDisabled) { ClearRuleProcessors(); GatherRuleProcessors(); return NS_OK; } The PresContext has to call this on its mStyleSet member before calling ReconstructFrames: NS_IMETHODIMP PresShell::StyleSheetDisabledStateChanged(nsIDocument *aDocument, nsIStyleSheet* aStyleSheet, PRBool aDisabled) { nsresult rv = NS_OK; // first notify the style set that a sheet's state has changed if (mStyleSet) { rv = mStyleSet->NotifyStyleSheetStateChanged(aDisabled ? PR_FALSE : PR_TRUE); } if (NS_FAILED(rv)) return rv; // rebuild the frame-world return ReconstructFrames(); } (I cannot easily create a patch right how as I have other unrelated changes in those files). I'm looking into the image problems now.

Cleaning up status whiteboard by marking this beta2 minus. (6/15 has passed) Folks are obviously too doomed for beta2 to get to this.

*** Bug 42971 has been marked as a duplicate of this bug. ***

Charley, I'll be checking in the fix for the Enable/Disable style sheet problem as part of a larger nsbeta2+ bug fix - it is clearly wrong and it is easy to fix (see related bug 38026). Once I do you can pull it and see that we are now correctly handling the icons coming and going, except that the images themselves are not always painting (their space is reserved however, and there is even a nice little border around where they should be when you select them).

Updating milestone...

Thanks, Marc. I'm building now and will evaluate where we are with remaining problems.

An interesting wrinkle: put some text into the doc, bold, italic, underlines, whatever. Switch to Show All Tags - you'll see the icons. Switch back to Normal, then back to Show All Tags - no icons now (the problem we have seen for the last couple of weeks). Now, select some of the text and change it to a new type, plie Heading 4 - voilla, the H4 icons show up. It appears any icon can be used once. As I noted before, the layout is leaving room for the icons, they are just not being rendered.

Changing to nsbeta3 since it was rejected for nsbeta2 (expired, actually).

Checked in the changes to make Enable/DisableStyleSheet actually rebuild the rule processors. Charley, can you check it out on your end? From Style's perspective, everything looks great. There seems to be a problem with the images not getting painted though.

As much as it pains me to do this, I am sending it back over to the Editor team now. I believe that the Style and Layout problems are taken care of, and now there are some residual painting or image or necko problems to tackle. Beppe, can you send it to the correct person (preferrably not one named Marc Attinasi)?

repaint issues -- I'm going to start with Kin and see what he thinks, then we can reassign it to the appropriate folks over in layout. Kin, can you help Charley to narrow down the repaint issues?

Accepting bug.

> then we can reassign it to the appropriate folks over in layout. > I don't think it is a Layout problem. Layout fulfilled its part: the space for these images is properly located on the page. We had similar problems (bug 41624 for instance) with images that don't always appear and they seem to come from somewhere between Cache and ImageLib, or maybe History.

The editor images are being marked as IC_INCOMPLETE by the image library when they are initially loaded because OnStopRequest() is called and a stream still exists: IL_StreamAbort(il_container_struct * 0x0355a400, int -201) line 1637 NetReaderImpl::StreamAbort(NetReaderImpl * const 0x0355be20, int -201) line 121 + 16 bytes ImageConsumer::OnStopRequest(ImageConsumer * const 0x0355bed0, nsIChannel * 0x03555060, nsISupports * 0x00000000, unsigned int 2147500037, const unsigned short * 0x00000000) line 537 nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x03554650, nsIChannel * 0x03555060, nsISupports * 0x00000000, unsigned int 2147500037, const unsigned short * 0x00000000) line 267 nsResChannel::EndRequest(unsigned int 2147500037, const unsigned short * 0x00000000) line 704 + 50 bytes nsResChannel::AsyncRead(nsResChannel * const 0x03555060, nsIStreamListener * 0x03554650, nsISupports * 0x00000000) line 414 nsResChannel::OnStopRequest(nsResChannel * const 0x03555064, nsIChannel * 0x0355bbd0, nsISupports * 0x00000000, unsigned int 2147500036, const unsigned short * 0x00000000) line 692 + 43 bytes nsFileChannel::OnStopRequest(nsFileChannel * const 0x0355bbd8, nsIChannel * 0x033900d0, nsISupports * 0x00000000, unsigned int 2147500036, const unsigned short * 0x00000000) line 628 + 45 bytes nsOnStopRequestEvent::HandleEvent(nsOnStopRequestEvent * const 0x033903f0) line 302 nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x03396f70) line 97 + 12 bytes PL_HandleEvent(PLEvent * 0x03396f70) line 575 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x01084060) line 520 + 9 bytes _md_EventReceiverProc(HWND__ * 0x00820096, unsigned int 49495, unsigned int 0, long 17317984) line 1032 + 9 bytes USER32! 77e7124c() 01084060() So the next time we recreate the image frames, we load the images out of the cache, but this time IL_GetImage() notices the IC_INCOMPLETE status, and sends it to nsImageRequest which converts it to an IL_ERROR_IMAGE_DATA_TRUNCATED error and then calls nsFrameImageLoader::NotifyError(), so the nsFrameImageLoader.mImage field never gets set, so there is nothing to paint. Here's the stack trace of the error thrown when we are loading the image the second time around: nsFrameImageLoader::NotifyFrames(int 0) line 537 nsFrameImageLoader::NotifyError(nsIImageRequest * 0x03539ee0, nsImageError nsImageError_kImageDataTruncated) line 529 ns_observer_proc(void * 0x0353b0a0, long 14, void * 0x0012e930, void * 0x03539ee0) line 151 XP_NotifyObservers(OpaqueObserverList * 0x0353b130, long 14, void * 0x0012e930) line 259 + 28 bytes il_icon_notify(_IL_ImageReq * 0x0353b0a0, int 17, long 14) line 522 + 20 bytes IL_GetImage(const char * 0x0353bcf0, _IL_GroupContext * 0x03559530, OpaqueObserverList * 0x0353b130, _NI_IRGB * 0x00000000, unsigned int 0, unsigned int 0, unsigned int 0, void * 0x03558200, nsIImageRequestObserver * 0x0353b274) line 2100 + 13 bytes ImageRequestImpl::Init(void * 0x03559530, const char * 0x0353bcf0, nsIImageRequestObserver * 0x0353b274, const unsigned int * 0x00000000, unsigned int 0, unsigned int 0, unsigned int 0, ilINetContext * 0x03558200) line 260 + 53 bytes ImageGroupImpl::GetImage(const char * 0x0353bcf0, nsIImageRequestObserver * 0x0353b274, const unsigned int * 0x00000000, unsigned int 0, unsigned int 0, unsigned int 0) line 266 + 46 bytes nsFrameImageLoader::Init(nsFrameImageLoader * const 0x0353b270, nsIPresContext * 0x033d3660, nsIImageGroup * 0x0355d030, const nsString & {...}, const unsigned int * 0x00000000, const nsSize * 0x00000000, nsIFrame * 0x02b15194, nsImageAnimation eImageAnimation_None, unsigned int (nsIPresContext *, nsIFrameImageLoader *, nsIFrame *, void *, unsigned int)* 0x00000000, void * ...) lin nsPresContext::StartLoadImage(nsPresContext * const 0x033d3660, const nsString & {...}, const unsigned int * 0x00000000, const nsSize * 0x00000000, nsIFrame * 0x02b15194, unsigned int (nsIPresContext *, nsIFrameImageLoader *, nsIFrame *, void *, unsigned int)* 0x00000000, void * 0x00000000, nsIFrameImageLoader * * 0x0012ebbc) line 970 + 62 bytes nsCSSRendering::PaintBackground(nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, nsIFrame * 0x02b15194, const nsRect & {...}, const nsRect & {...}, const nsStyleColor & {...}, const nsStyleSpacing & {...}, int 0, int 0) line 2101 + 76 bytes nsBlockFrame::Paint(nsBlockFrame * const 0x02b15194, nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line 6263 + 37 bytes nsContainerFrame::PaintChild(nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsIFrame * 0x02b15194, nsFramePaintLayer eFramePaintLayer_Underlay) line 211 nsBlockFrame::PaintChildren(nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line 6408 nsBlockFrame::Paint(nsBlockFrame * const 0x02b14f38, nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line 6286 nsContainerFrame::PaintChild(nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsIFrame * 0x02b14f38, nsFramePaintLayer eFramePaintLayer_Underlay) line 211 nsBlockFrame::PaintChildren(nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line 6408 nsBlockFrame::Paint(nsBlockFrame * const 0x02b14eec, nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line 6286 nsContainerFrame::PaintChild(nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsIFrame * 0x02b14eec, nsFramePaintLayer eFramePaintLayer_Underlay) line 211 nsContainerFrame::PaintChildren(nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line 155 nsHTMLContainerFrame::Paint(nsHTMLContainerFrame * const 0x02b14224, nsIPresContext * 0x033d3660, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line 101 PresShell::Paint(PresShell * const 0x034665e4, nsIView * 0x034c0ab0, nsIRenderingContext & {...}, const nsRect & {...}) line 3657 + 34 bytes nsView::Paint(nsView * const 0x034c0ab0, nsIRenderingContext & {...}, const nsRect & {...}, unsigned int 128, int & 268588693) line 280 nsViewManager2::RenderDisplayListElement(DisplayListElement2 * 0x034c2cd0, nsIRenderingContext & {...}) line 816 nsViewManager2::RenderViews(nsIView * 0x0310e300, nsIRenderingContext & {...}, const nsRect & {...}, int & 0) line 763 nsViewManager2::Refresh(nsIView * 0x0310e300, nsIRenderingContext * 0x035386f0, const nsRect * 0x0012f6d8, unsigned int 1) line 643 nsViewManager2::DispatchEvent(nsViewManager2 * const 0x03466e20, nsGUIEvent * 0x0012f818, nsEventStatus * 0x0012f71c) line 1303 HandleEvent(nsGUIEvent * 0x0012f818) line 69 nsWindow::DispatchEvent(nsWindow * const 0x03125ef4, nsGUIEvent * 0x0012f818, nsEventStatus & nsEventStatus_eIgnore) line 560 + 10 bytes nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f818, nsEventStatus & nsEventStatus_eIgnore) line 586 nsWindow::OnPaint() line 3487 + 28 bytes nsWindow::ProcessMessage(unsigned int 15, unsigned int 0, long 0, long * 0x0012fbc8) line 2647 + 17 bytes nsWindow::WindowProc(HWND__ * 0x00d103ea, unsigned int 15, unsigned int 0, long 0) line 829 + 27 bytes USER32! 77e71303() USER32! 77e71962() NTDLL! 77f763ef() I'm not sure if our gifs are invalid, or if this is a bug in necko.

It looks like nsFileTransport::Process() is firing off an OnStopRequest() in the END_READ case. Necko assumes that if a stream exists and an OnStopRequest is fired, that an abort is taking place. This is probably the same thing that is causing bug #40785 which I just assigned to dougt@netscape.com.

The fix for #40785 fixes this bug as well. Will check in as soon as tree opens. -p

fix checked in. -pn

verified in 7/13 build.

Adding keyword to bugs which already show a nsbeta2 triage value in the status whiteboard so the queries don't get screwed up.