476357 - The zooming function on the page makes the text overlap

Status: RESOLVED FIXED

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

Description

[levi.ustinov]

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.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Confirmed on Windows XP. This is also a regression, regressed somewhere in November 2007.

Comment 2

[Ria Klaassen (not reading all bugmail)]

Regression range: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2007-11-29+13%3A00&maxdate=2007-11-29+19%3A00
Maybe caused by bug 405133 ?

Comment 3

[Philip Chee]

> Maybe caused by bug 405133 ?
Highly unlikely. The reporter is using Firefox 3.0.5. b405133 is SeaMonkey specific and NPOTFB.

Comment 4

[Ria Klaassen (not reading all bugmail)]

Then there is only one other possibility: Bug 320378.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Need a testcase.

If we can get a fix we might want to take it for 1.9.1.

Comment 6

[levi.ustinov]

(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 :).

Comment 7

[Matthias Versen [:Matti]]

>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,....

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: 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?).

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

My guess is that this is the same underlying bug as bug 363247, which is really a pretty bad bug...

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, it doesn't seem to be the same.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: mostly simplified

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This partial reversion:
-    if (line->IsDirty() && (line->HasFloats() || !willReflowAgain)) {
+    if (line->IsDirty()) {
makes the bug go away.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: simplified

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

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...

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Full reftests do pass.)

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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...

Comment 18

[Boris Zbarsky [:bzbarsky]]

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).

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+      // 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?

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(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.)

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 23

[Boris Zbarsky [:bzbarsky]]

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.

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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?

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 360605 [details] [diff] [review]
patch

No, this is cleaner after all. Sorry about the delay.

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/30fd045df716

Comment 27

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1c6f10de1f4a