Closed Bug 84046 Opened 23 years ago Closed 23 years ago

Dynamic changes to stylistic attributes do not take effect

Categories

(Core :: Layout, defect, P2)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla0.9.2

People

(Reporter: cmanske, Assigned: hyatt)

References

(
URL
)

Details

(Keywords: regression, testcase, Whiteboard: [Hixie-P2] approved patch, checkin ETA?)

Attachments

(1 file)

Fix bug.,
1.18 KB, patch
Details | Diff | Splinter Review 
Setting "bgcolor" attribute for the page or on the <table> or <td> elements
doesn't display at all in Composer. But if you save to a file, then view the page
in browser or reload into editor, color shows ok.
Simple test: Start Composer with any file or new page. Click on lower-right
square of colorpicker on toolbar. Select a color for the background.
This must be fixed by RTM!
Severity: normal → critical
Keywords: correctness, regression
*** Bug 83719 has been marked as a duplicate of this bug. ***
May be related to the fix for bug 75591.
This is working fine for me on my CVS build from this morning...

The page background works as I'd expect, but the table background messed me up 
for a second because you have to check the little checkbox in addition to 
picking the color.

Can others double-check this? If this is a bug, I agree it is serious, but I 
cannot see it.
I'm pulled again and am rebuilding now to get latest changes...
Its working correctly for me on WINNT with 2001060409 build.
Weird. I restored the composer CSS files (had Daniels's new ones before), but I
still see the problem, including page background not changing. The only way I
can get the page background to display is to go into "Show All Tags" mode.
I'm updating+building again now, and will examine all changes in my tree to try
to find out what's happening.
Concerning selecting a color in Table Dialog, you should be able to pick a color
via the button, and the checkbox should check automatically. If it doesn't, that's
a bug.
I still see no color change when setting page or table backgrounds using my
debug build from today. I can see the color change with I switch to "Show all
tags mode", and it is retained after returning to normal mode. (Mode switching
adds, removes an override stylesheet.) But after returning from "All Tags" mode,
 the yellow "body" icon still remains. We used to often have this bug during
earlier layout development. Could Hyatt's CSS optimization have caused these
problems?

Ok! We've figured out why some do and some don't see the bug. The color fails
to display only after setting > 1 times. So to test, change to one color, then
immediately change it to a different color.
The reason I didn't see the color change the "first time" was because there's
a pref to set initial page colors when creating a new page (default is don't do
that). So the bug is that once the 'bgcolor' is set on the <body>, it can't be
changed to another value.
And it seems that setting the "bgcolor" is what is also triggering the problems
with setting it for table elements.


I just changed the page background color 5 times, it showed up all 5 times.
Also, I changed the background color on the table cell 5 times and that is fine
too. I cannot, however, see color changes on the table at all, at least not
until I open the file in the browser.


Charley, what's the scoop here? I just tried again with a 06/07 build and it is
OK. I'd like to mark thos WORKSFORME, but I want to make sure you agree first.
I don't know what to say other than I definitely still see exactly the problem
I described! I can only change the color once.
I've emailed this to entire editor group, so can you hold off for a day or so
for more confirmation?
Thanks

when I open new blank page and immediately change the background
color from the toolbar, I see the new bg color.

Shrirang, can you try also? thanks...
This is OK on my Linux and Mac builds too, BTW.

Petersen, can you please test this out too?
my windows branch works fine  0607.
And everyone is setting the color multiple times?
*raises hand* Yes!
*raises both hands* yes!

Charley, do you have a second machine to test on? If not, can you create a new 
profile and try that? Something interesting is happening on your installation it 
seems...
This seems to work fine on Windows ME. I can change and apply a color to the
table or td element. Tested on June 07th build (2001060713)
interesting....
this is *NOT* working for me on my Mac mozilla debug build (from today).

This is what I am doing:
 * new composer document
 * insert table (2x2)
 * place caret in table
 * go to Table menu and choose Select | Table
 * click the bottom/right color picker
 * pick the light-blue color
 * notice no color change to table
 * go to html source mode and see "bgcolor" on <table>
OS: Windows NT → All
Hardware: PC → All
I can confirm this behavior too, using yesterday's build. The background color
on the page is still fine, but the table is not working now. Strangely, changing
the default 2X2 table's background colors a couple of times cause the size of
the table to change too - it got really really small (markup still said width=100%).

NS6.1 BETA1 works fine.
The table re-sizing problem is bug 84018, which doesn't have a milestone and
IMHO, must be fixed! Basically, I think table layout is currently a mess;
lots of random lost borders and resizing problems.
It will be a real shame if we ship like this!

So, On Windows 2K and Mac (9.1) I get the same behavior in yesterday's build. I
can change the color of the table cell if I click in a cell and choose Format |
Table Cell Properties and choose a color, but the table size shrinks (width=100%
is being ignored after I change a cell's background color). I'll try and get
some help from the table-lord...

Also, updating summary and accepting since I can reproduce it now. Note that I
have not seen anyone except Charley report a problem with page background
colors, so clearing that from the summary.
Status: NEW → ASSIGNED
Summary: Setting background color for page or tables doesn't display in Composer → Setting background color for tables doesn't display in Composer
This is pretty serious, and apparently recent (for me at least). Moving to 0.9.2
Priority: -- → P2
Target Milestone: --- → mozilla0.9.2
This is looking like a regression from Hyatt's Rule matching changes that he
committed on 5/31 - the 5/31 build is good, the 6/1 build is bad. CC'ing Hyatt
to see if he can help out.
I can take this if you want to reassign it to me, Marc.

OK, Thanks David. Reassigning and CC'ing myself so I can keep in touch with 
was'up
Assignee: attinasi → hyatt
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Taking QA (style branch fallout).

Hyatt: This doesn't work for DHTML either, you were right. See:
   http://www.hixie.ch/tests/adhoc/dom/html/attributes/001.xml
   http://www.hixie.ch/tests/adhoc/dom/html/attributes/002.xml
URL: http://www.hixie.ch/tests/adhoc/dom/h...
Severity: critical → major
Keywords: mozilla0.9.2, testcase
QA Contact: petersen → ian
Summary: Setting background color for tables doesn't display in Composer → Dynamic changes to stylistic attributes do not take effect
Whiteboard: [Hixie-P2]
Attached patch Fix bug., — — Splinter Review 

    
    

    
  
This is a pretty easy fix.  Here's what was happening.  Attributes in HTML that 
affect style are bundled collectively into a mapped attributes object.  For 
example, in

<font color="green" size="3">

The two attributes and their values together comprise a single mapped 
attributes object.  This object functions as a style rule (it implements 
nsIStyleRule) and supplies style data for the rule matching code.

If multiple objects on a page have the same set of attributes and values, then 
they share a mapped attribute object, e.g., if several table cells all said

<td bgcolor="white">

There is only one mapped attribute object that represents bgcolor="white".  
This causes me to achieve great sharing in my rule tree, since these elements 
all use the same style rule (and therefore share a rule node in the rule tree).

What caused problems was an optimization in the mapped attributes code.  If the 
code saw that only one element was making use of a set of mapped attributes it 
would mutate the object in place, in effect modifying the contents of a style 
rule.

Since I cache computed data in my rule tree, for this case, the rule tree 
became invalid.

My patch removes this optimization, as it isn't that common a case anyway, and 
the cost to clone these objects is cheap anyway.


    
    

    
  
r=cmanske


    
    

    
  
Thanks David, and thanks too for the nice explanation! sr=attinasi

    
    

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

    
    

    
  
a=dbaron for trunk checkin (on behalf of drivers)

    
    

    
  
When is this going in?
Whiteboard: [Hixie-P2] → [Hixie-P2] approved patch, checkin ETA?

    
    

    
  
It went in yesterday.  JUst forgot to close it.

Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: