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)

defect

Tracking

()

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: 25 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: 25 years ago25 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: beta1beta2
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: 25 years ago24 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 ago24 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 ago24 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.