Closed Bug 476357 Opened 11 years ago Closed 11 years ago

The zooming function on the page makes the text overlap

Categories

(Core :: Layout: Block and Inline, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1b3

People

(Reporter: levi.ustinov, Assigned: dbaron)

References

()

Details

(Keywords: fixed1.9.1, regression)

Attachments

(4 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 (.NET CLR 3.5.30729)

The zoom-in function for this page is located on the top right corner of the page. When you increase the text size using this function, the text overlaps (when the zooming gets too big).

I have confirmed, hat in Google Chrome (latest stable build) the problem does NOT appear, thus it is not a problem with the website.

I have full access to the website and the host (since I am the head administrator of that network). The server is running one of the latest version of Joomla! and is using a freeware theme.

If it is needed I  can provide more detailed information from the server side.

Reproducible: Always

Steps to Reproduce:
1.Hold down your mouse pointer on the slider, located on the top right corner of the web page.
2.Slide the slider towards the right to make the text bigger.
3.See the text become bigger and overlap each other.
Actual Results:  
The text overlaps and is not readable; page is not displayed correctly.

Expected Results:  
The text to grow bigger and advance down the table.

The only add-ons I was using is GreaseMonkey with a simple link (dead or not) checker and some Microsoft latest Framework add-on which seems to install itself upon Windows Updates.  Thought the problem occurred before with no add-ons.
Confirmed on Windows XP. This is also a regression, regressed somewhere in November 2007.
Component: General → Layout
Keywords: regression
Product: Firefox → Core
QA Contact: general → layout
Version: unspecified → Trunk
Status: UNCONFIRMED → NEW
Ever confirmed: true
> Maybe caused by bug 405133 ?
Highly unlikely. The reporter is using Firefox 3.0.5. b405133 is SeaMonkey specific and NPOTFB.
Then there is only one other possibility: Bug 320378.
Need a testcase.

If we can get a fix we might want to take it for 1.9.1.
Flags: wanted1.9.1?
(In reply to comment #5)
> Need a testcase.
> 
> If we can get a fix we might want to take it for 1.9.1.

I am sorry, this is my first time reporting a bug, so can you please explain what a testcase is :(. And take it for what? Is that a Firefox version :).
>I am sorry, this is my first time reporting a bug, so can you please explain
>what a testcase is :(. 

A testcase is the given URL but I think roc wants a minimized/reduced testcase that only contains the minimal html to see the issue and not all not necessary Javascript and CSS.
Read https://developer.mozilla.org/en/Reducing_testcases for more informtation

>And take it for what? Is that a Firefox version :).
Firefox doesn't render or display anything, that is done the rendering Engine called Gecko in the Mozilla case or KHTML for Safari/Google Chrome. Only the menu, bookmarks etc. are Firefox.
If you look at your initial comment#0 you should see the used Gecko version in the Browser User Agent string for FF3.0.5. Gecko 1.9.1 will be used by Firefox 3.1,Seamonkey2, Thunderbird3,....
Attached file partially simplified
This is simplified down to being in one file, but still pretty complicated.

I replaced the slider with a setTimeout() that does what the slider does, so that the problem shows up 200ms after loading.

This may be simplified enough... it's clear that what the slider is doing is changing the font-size property of an element, and that's somehow not causing a full reflow of the subtree (maybe due to not propagating into out-of-flows?).
My guess is that this is the same underlying bug as bug 363247, which is really a pretty bad bug...
Actually, it doesn't seem to be the same.
From the look of it, this is a regression from bug 320378.  I think what's happening is that we're bailing during a reflow when we're marked dirty, and then when we redo we're not marked dirty, so we don't reflow all the children, which the dirty bit requires.
Blocks: 320378
Severity: minor → normal
Component: Layout → Layout: Block and Inline
Priority: -- → P3
QA Contact: layout → layout.block-and-inline
This partial reversion:
-    if (line->IsDirty() && (line->HasFloats() || !willReflowAgain)) {
+    if (line->IsDirty()) {
makes the bug go away.
OS: Windows Vista → All
Hardware: x86 → All
No longer depends on: 363247
Flags: blocking1.9.1?
Flags: blocking1.9.1? → blocking1.9.1+
Priority: P3 → P2
Attached patch patchSplinter Review
This one is a little scary; please review carefully.

I also wonder how this problem interacts with bzbarsky's interruptible reflow work.  I suppose I should look at his patch...
Attachment #360605 - Flags: superreview?(roc)
Attachment #360605 - Flags: review?(roc)
(Full reftests do pass.)
Comment on attachment 360605 [details] [diff] [review]
patch

>+    if (blockHtmlRS.mDiscoveredClearance && *blockHtmlRS.mDiscoveredClearance) {

I'll change this to blockHTMLRS.WillReflowAgainForClearance().  I forgot that it was available...
I'm not sure it really interacts at all with the current interruptible reflow approach (which is to not remove any dirty bits after we interrupt).
+      // If an ancestor of ours is going to reflow for clearance, we
+      // need to avoid calling PlaceBlock, because it unsets dirty bits
+      // on the child block (both itself, and through its call to
+      // nsFrame::DidReflow), and those dirty bits imply dirtiness for
+      // all of the child block, including the lines it didn't reflow.

But when that child block did ReflowDirtyLines, wouldn't it have had selfDirty set to true, in which case all the lines would have been marked dirty during the ReflowDirtyLines main loop?
(In reply to comment #19)
> But when that child block did ReflowDirtyLines, wouldn't it have had selfDirty
> set to true, in which case all the lines would have been marked dirty during
> the ReflowDirtyLines main loop?

Line dirtiness doesn't imply frame dirtiness.  (Line dirtiness is also much more analogous to nsIFrame's has-dirty-children bit than nsIFrame's is-dirty bit.)

(That said, Boris is probably going to have to make dirty frames mark all their kids dirty for the interruptible reflow work.)
Whiteboard: [needs review]
True. Then I think that if selfDirty is set and we skip reflowing a dirty line, we should mark all frames on that line dirty.
Even so, I think we probably still need to skip clearing the dirty-children bit, particularly for the case where we have multiple levels of nesting around the source of clearance.
For what it's worth, right now the way I'm trying to do interruptible reflow is that:

1)  If I detect an interrupt I mark all lines after that point dirty.
2)  If NS_FRAME_IS_DIRTY is set I set it on all the kids of lines after the
    interrupt point.
3)  I mark all lines of descendant blocks on lines after the interrupt dirty
    (basically to deal with float damage; I have an XXX comment to mark less
    dirty there).
4)  I add the block in whose line list we interrupted to a list on presshell.
    Once we get back to DoReflow, all frames in that list and all their
    non-dirty ancestors are marked with NS_FRAME_HAS_DIRTY_CHILDREN.  The
    relevant reflow root (which therefore has NS_FRAME_HAS_DIRTY_CHILDREN) is
    added back to the dirty roots array.
Sounds good Boris.

(In reply to comment #22)
> Even so, I think we probably still need to skip clearing the dirty-children
> bit, particularly for the case where we have multiple levels of nesting around
> the source of clearance.

Good point.

OK, so how about as a safer fix, let the call to PlaceBlock happen but if we've discovered clearance, re-add DIRTY/HAS_DIRTY_CHILDREN to the frame after calling PlaceBlock?
Attachment #360605 - Flags: superreview?(roc)
Attachment #360605 - Flags: superreview+
Attachment #360605 - Flags: review?(roc)
Attachment #360605 - Flags: review+
Comment on attachment 360605 [details] [diff] [review]
patch

No, this is cleaner after all. Sorry about the delay.
http://hg.mozilla.org/mozilla-central/rev/30fd045df716
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [needs review] → [needs 1.9.1 landing]
Target Milestone: --- → mozilla1.9.2a1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1c6f10de1f4a
Keywords: fixed1.9.1
Whiteboard: [needs 1.9.1 landing]
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
Depends on: 513394
You need to log in before you can comment on or make changes to this bug.