Closed Bug 424631 Opened 16 years ago Closed 16 years ago

Inconsistent rendering with RLO

Categories

(Core :: Layout: Text and Fonts, defect, P1)

Product:
Core
Component:
Layout: Text and Fonts
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9

People

(Reporter: jruderman, Assigned: uriber)

References

Details

(Keywords: regression, testcase)

Attachments

(4 files, 1 obsolete file)

testcase (dynamic)
225 bytes, text/html
Details
reference (static)
162 bytes, text/html
Details
patch
1.93 KB, patch
smontagu
: review+
dbaron
: superreview+
Details | Diff | Splinter Review
Jesse's testcase as a reftest patch
1.88 KB, patch
Details | Diff | Splinter Review 
The testcase appears as "abcd" and the reference appears as "abdc".  They should look the same, since they have the same final DOM, but I don't know which rendering is correct.

Bug 424629 might be related.
Attached file testcase (dynamic) — 

    
    

    
  

      
      
      
      

        Attached file
          
        reference (static)
        — 
      

    


  

    
    

    
  
Reversing the checkin from bug 423130 fixes this.
Blocks: 423130

    
    

    
  
This might actually have some very bad effects in more common cases. Patch coming up.
Severity: minor → normal
Flags: blocking1.9?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter Review
      

    


  
So, of course, we should only join a frame's parent inline as long as the frame is the last child of that parent.

Sorry for getting this wrong the first time. I verified that this fixes this bug and does not regress bug 423130.
Assignee: nobody → uriber
Status: NEW → ASSIGNED

        Attachment #311649 -
        Flags: superreview?(dbaron)

        Attachment #311649 -
        Flags: review?(smontagu)

    
  
Keywords: regression

    
    

    
  

      
      
      
      

        Attached patch
          
          
        alt patch (obsolete)
        — 
          — Splinter Review
      

    


  
This might be considered a bit cleaner (behavior should be the same). Take your pick.

        Attachment #311655 -
        Flags: superreview?(dbaron)

        Attachment #311655 -
        Flags: review?(smontagu)

    
    

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

I like this one better

        Attachment #311649 -
        Flags: review?(smontagu) → review+

    
    

    
  
Comment on attachment 311655 [details] [diff] [review]
alt patch

OK, then.

        Attachment #311655 -
        Attachment is obsolete: true

        Attachment #311655 -
        Flags: superreview?(dbaron)

        Attachment #311655 -
        Flags: review?(smontagu)

    
    

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

sr=dbaron.  Be sure to check in the reftests along with the patch.  (There are reftests for the patch that caused this regression, right?)

        Attachment #311649 -
        Flags: superreview?(dbaron) → superreview+

    
  

        Attachment #311649 -
        Flags: approval1.9b5?

        Attachment #311649 -
        Flags: approval1.9?

    
    

    
  
Yes, bug 423130 has a reftest, and I'll make one for this bug as well.

    
    

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

beta 5 is closed, moving approval flag ..

        Attachment #311649 -
        Flags: approval1.9b5?
Blocks: 425338

    
    

    
  
This may become a stop-ship bug for b5 -- this patch also needs a reftest or other appropriate test.  Can someone cook one up?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1

    
    

    
  
Vlad, the two files I attached when I filed this bug are a reftest.

    
    

    
  


  
This is what I'll check in as a reftest.

    
    

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

I guess I don't need approval1.9 since this is a blocker.

        Attachment #311649 -
        Flags: approval1.9?

    
    

    
  
Checked in with reftest.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED

    
  
Target Milestone: --- → mozilla1.9

    
    

    
  
This is not fixed. Trying to verify the testcase and reference attachment shows only "abc" for the former one. As comment 0 states, both should look identical.

Tested with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko Minefield/3.0pre ID:2008032723
Status: RESOLVED → REOPENED
Hardware: PC → All
Resolution: FIXED → ---

    
    

    
  
Sorry, noticed that Javascript was deactivated due to a former test.

I can verify the fix.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED

    
  
Hardware: All → PC
Target Milestone: mozilla1.9 → ---

    
  
Target Milestone: --- → mozilla1.9

    
  
OS: Mac OS X → All
Hardware: PC → All

    
  
Blocks: 424629

    
  
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: layout.bidi → layout.fonts-and-text

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: