Closed Bug 24343 Opened 25 years ago Closed 24 years ago

Changing font/size pref causes garbled display

Categories

(Core :: CSS Parsing and Computation, defect, P3)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: pierre, Assigned: sfraser_bugs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [nsbeta2+][7/22]) 

I'm opening this bug following a discussion I had with Erik.
- go to any web page
- open the pref dialog and change the font size
- click ok to dismiss the dialog
==> The display is all garbled. A resize doesn't help, you have to reload the
page to fix the problem.

The bug is in nsPresContext::PreferenceChanged()/PresShell::StyleChangeReflow().
Status: NEW → ASSIGNED
We'd like to get this fixed for M14/Beta1, to make the font prefs usable.
Target Milestone: M13
It should have been M13. I forgot to set the target milestone when I opened the
bug.
Not critical to M13.  Moving to M14 per today's PDT mtg.
Target Milestone: M13 → M14
*** Bug 25934 has been marked as a duplicate of this bug. ***
*** Bug 25967 has been marked as a duplicate of this bug. ***
fyi, thanks to attinasi (see bug 13693 and bug 25425) part of the problem was 
fixed: the table borders no longer appear when dismissing the dialog (see bug 
25934). We still have an incorrect display: the text size changes but not the 
overall size of the objects.

This is needed for Beta1 to make the font prefs UI work as the user expects
(without a manual reload of the page).
Keywords: beta1
PDT+
Whiteboard: [PDT+]
I've been looking at this bug. It looks like it is not reflowing the frames when 
the preferences are changed.  
*** Bug 26799 has been marked as a duplicate of this bug. ***
Blocks: 25427
As Pierre suggested, reflowing the frames isn't enough. We need to re-resolve 
the style for all of the frames, or simpler still, recreate the frames 
altogether. I have tried reconstructing the frame-tree using 
ReconstructDocElementHierarchy on the StyleSet, but subsequently there are 
assertions and eventually a crash trying to : GlobalWindowImpl::GetTreeOwner is 
failing because the treeOwner is set to null when the HTMLElement's frame is 
destroyed. I'm trying now to re-establish the owner... any ideas?
I think I have a way to fix this: by marking the rootframe dirty and doing a 
dirty-reflow. I think this will work we get the StyleChange reflow working in 
all of the frames (some appear to not support eReflowReason_StyleChange). Kevin 
and I have seen that approach used in other similar cases.
Assignee: pierre → attinasi
Status: ASSIGNED → NEW
I cannot get it to work... Given our limited time constraint it is probably best 
if Troy looks at it.

A couple of things I have found:
1) PresShell::StyleChangeReflow() needs to set the reflowstate reason to 
eReflowReason_StyleChange, not eReflowReason_Resize
2) nsPresContext::PreferenceChanged is called at startup many, many times, which 
may be slowing down startup. After the pref is changed it is called for all of 
the PresContexts, which may also be unnecessary (only the document really needs 
to get the notification I think).
3) nsBlockFrame doesn't appear to handle the eReflowReason_StyleChange
I hope this information helps...
Assignee: attinasi → troy
> 2) nsPresContext::PreferenceChanged is called at startup many, many times,
> which may be slowing down startup.

Yes, I'm concerned about this too. It would be nice if we had some way of
"batching" the changes. I.e. turn off reflow, call all callbacks, and then turn
on reflow again, similar to some of the other batching that Gecko does.

> After the pref is changed it is called for all of 
> the PresContexts, which may also be unnecessary (only the document really
> needs to get the notification I think).

It is true that the charset (and language group) are "per-document", but the
font metrics objects are stored in device contexts. How many device contexts
are there per document? One per presentation context? We need to destroy (or
update) each font metrics object for every pref change.
I don't think we should be trying to fix this for beta1. There are too many 
issues to get right to think that we won't end up causing regressions

Removing PDT+ designation, please reconsider whether this should be fixed for 
beta1
Whiteboard: [PDT+]
Whiteboard: [PDT-]
Status: NEW → ASSIGNED
*** Bug 25705 has been marked as a duplicate of this bug. ***
*** Bug 30485 has been marked as a duplicate of this bug. ***
Fixed several problems:
- pres shell StyleChangeReflow() function now passes eReflowReason_StyleChange 
as reflow reason
- block frame and table frame code now handle eReflowReason_StyleChange 
correctly
- viewport frame repaints the visible area after processing the reflow
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Thanks Troy, I don't know what bad things we would have done without you.

*** Bug 29729 has been marked as a duplicate of this bug. ***
Troy, I don't know what happened, but we have a regression in this area. I don't
think that I caused it by factoring out the code to create RemapStyleAndReflow.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 35012
I think the problem is in the box frame code. The pres shell's 
StyleChangeReflow() code gets called and the viewport frame is reflowed with a 
reflow reason of eReflowReason_StyleChange which is correct

The viewport frame reflows its child frame with the eReflowReason_StyleChange 
reflow reason as well, but no block frame code isn't called which says that the 
reflow is getting squelched somewhere between the viewport and the HTML 
element's frame

Looking at the box frame code it doesn't explicitly handle 
eReflowReason_StyleChange and so it ends up treating it like a reflow dirty 
which is wrong and I suspect that is the problem
Assignee: troy → evaughan
Status: REOPENED → NEW
Status: NEW → ASSIGNED
Whiteboard: [PDT-] → [PDT-]have fix
Target Milestone: M14 → M16
I have a fix for this. Your right troy I was not handling style change reflow 
correctly. It seems to work pretty well now so I'll check in my fixes as soon as 
the tree opens.

*** Bug 35575 has been marked as a duplicate of this bug. ***
fixed
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Using 4/13 build, verified fixed.
Status: RESOLVED → VERIFIED
chrisd, just curious: were you using m15 or m16 bits? thx!
Can't verify on Linux with Build 2000041309.  The problem still exists exactly
as described by the original reporter.
I'm seeing this behavior on Mac M16-041309.

Eric, perhaps this bug re-appeared because your change was backed out?
Status: VERIFIED → REOPENED
Keywords: beta1 → beta2
Resolution: FIXED → ---
Whiteboard: [PDT-]have fix
Keywords: nsbeta2
Putting on [nsbeta2+] radar.
Keywords: beta2
Whiteboard: [nsbeta2+]
Blocks: 34271
This is fixed. You may for a moment see the font change and overlap but then the 
page relays out correctly.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Works on:
 - Linux6 2000-05-23-10-M16 Commercial Build
 - Win98 2000-05-23-10-M16 Commercial Build

Does not work on:
 - MacOS9 2000-05-23-12-M16 Commercial Build

The text size does not change on the macos platform.  Re-opening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Note that we already have a bug opened for the Mac font prefs: bug 38318. Gary,
I'm reassigning this bug to you. Once you have fixed the Mac font prefs, please
confirm that this bug has been fixed too, and mark it FIXED. Thanks.
Assignee: evaughan → garywade
Status: REOPENED → NEW
Thank you Eric - info noted and 'depends on' updated.
Depends on: 38318
reassign sfraser.
sfraser- if this a dup of your fix yesterday, please mark it fixed. Otherwise, 
reassign this back to  garywade@desisoftsystems.com
Assignee: garywade → sfraser
Yup, this is fixed now.
Status: NEW → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Re-opening on:
- MacOS9 2000-06-30-09-M17 Commercial
- Linux6 2000-06-30-08-M17 Commercial
- Win98  2000-06-30-09-M17 Commercial

When I load voodoolady.mcom.com and perform the steps to reproduce listed above, 
I get javascript garbage at the bottom of the screen on all three platforms...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Chris, I'm really out of luck today to reproduce the bugs you are reopening. This 
one too works for me on the Mac with a 06/30 M17 build.

added fix by date in status field
Whiteboard: [nsbeta2+] → [nsbeta2+][8/2]
setting milestone to m17
Whiteboard: [nsbeta2+][8/2] → [nsbeta2+][7/22]
Target Milestone: M16 → M17
ckritzer: please re-verify, since the JS garbage could have been caused by 
another bug (rickg's parser bustage).
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
I'm on it...

Marking VERIFIED FIXED on:
- MacOS9 2000-07-11-08-M17 Commercial
- Linux6 2000-07-11-08-M17 Commercial
- Win98  2000-07-11-08-M17 Commercial


Looks great!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.