Closed Bug 1388877 Opened 7 years ago Closed 7 years ago

stylo: Some failing first-line reftests

Categories

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

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: manishearth, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Testcase
289 bytes, text/html
Details
Bug 1388877. Fix insertions under a ::first-line in stylo.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
We still have some failing first-line reftests:

./layout/reftests/bugs/reftest.list:fails-if(styloVsGecko) == 495385-5.html 495385-5-ref.html
./layout/reftests/details-summary/reftest.list:fails-if(styloVsGecko) == details-first-line.html details-first-line-ref.html
./layout/reftests/details-summary/reftest.list:fails-if(styloVsGecko) == open-details-first-line-1.html open-details-first-line-ref.html
./layout/reftests/details-summary/reftest.list:fails-if(styloVsGecko) == open-details-first-line-2.html open-details-first-line-ref.html


The first one seems to have to do with dynamic changes and restyling. The other three are bugs with it being used with <details>/<summary>
Flags: needinfo?(bzbarsky)
> ./layout/reftests/bugs/reftest.list:fails-if(styloVsGecko) == 495385-5.html 495385-5-ref.html

There's a good chance this one is bug 1385656.  In fact, I'm about 95% sure of it.  I'll know for sure tomorrow when I fix it.

> ./layout/reftests/details-summary/reftest.list:fails-if(styloVsGecko) == details-first-line.html details-first-line-ref.html
> ./layout/reftests/details-summary/reftest.list:fails-if(styloVsGecko) == open-details-first-line-1.html open-details-first-line-ref.html
> ./layout/reftests/details-summary/reftest.list:fails-if(styloVsGecko) == open-details-first-line-2.html open-details-first-line-ref.html

These are all due to a Gecko bug.  Specifically, bug 520605.

It was actually easier to fix it in the stylo first-line implementation than to reproduce it.  This is also the bug demonstrated by layout/reftests/first-line/ib-split-1.html and mentioned in the commit message of https://hg.mozilla.org/mozilla-central/rev/d5d6e006b251
Depends on: 1385656
So one option is to rewrite the references for these tests so that we have failures if !stylo || styloVsGecko, to make it clear that it's a Gecko issue.
Nah, we'll just wait for that bug to be fixed. If it's probably a gecko issue, no hurry.
Priority: -- → P4
To be clear, I don't think it's going to get fixed in Gecko's style system at this point.  Turning on stylo is going to be the fix. ;)
> There's a good chance this one is bug 1385656.  In fact, I'm about 95% sure of it.

Well, I was wrong.  It's a stylo first-line bug.  I'll attach a pretty minimal testcase without first-letter involved.  Working on this now.
Priority: P4 → P2
Attached file Testcase — 
Flags: needinfo?(bzbarsky)

    
    

    
      
  


  
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED

    
    

    
  
Comment on attachment 8896124 [details]
Bug 1388877.  Fix insertions under a ::first-line in stylo.

https://reviewboard.mozilla.org/r/167404/#review172618

::: layout/base/nsCSSFrameConstructor.cpp:11743
(Diff revision 1)
> +  // Check whether there's a ::first-line on the path up from aParentFrame to
> +  // aContainingBlock.

If you want another quick early exit, you could check for StyleContext()->HasPseudoElementData().  (I think that's set correctly for ServoStyleContexts too.)

        Attachment #8896124 -
        Flags: review?(cam) → review+

    
    

    
      
  

    
    

    
  
Comment on attachment 8896124 [details]
Bug 1388877.  Fix insertions under a ::first-line in stylo.

https://reviewboard.mozilla.org/r/167404/#review172624

::: layout/base/nsCSSFrameConstructor.h:2061
(Diff revisions 1 - 2)
> -   * aContainingBlock may have a first-line frame, it's possible that we're
> -   * inserting under that first-line frame.  In that case, with servo's style
> -   * system, the styles we resolved for aFrameItems are wrong (they don't take
> -   * ::first-line into account), and we should fix them up.  This method does
> -   * not mutate aFrameItems.
> +   * pseudo-element-affected styles, it's possible that we're inserting under a
> +   * ::first-line frame.  In that case, with servo's style system, the styles we
> +   * resolved for aFrameItems are wrong (they don't take ::first-line into
> +   * account), and we should fix them up, which is what this method does.
> +   *
> +   . This method does not mutate * aFrameItems.

./*

    
    

    
      
  

    
    

    
  
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c2389558858
Fix insertions under a ::first-line in stylo.  r=heycam

    
    

    
  
Backed out for asserting at ServoRestyleManager.cpp:1450 in stylo reftests and crashtests:

https://hg.mozilla.org/integration/autoland/rev/16f4c4efa1485881bee689a17e654ac39d8627fa

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=6c23895588581092613c8d6a054bfe54478f1462&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=122498018&repo=autoland

[task 2017-08-11T08:18:11.806658Z] 08:18:11     INFO - REFTEST TEST-START | file:///home/worker/workspace/build/tests/reftest/tests/layout/reftests/bugs/291078-1.html == file:///home/worker/workspace/build/tests/reftest/tests/layout/reftests/bugs/291078-1-ref.html
[task 2017-08-11T08:18:11.807510Z] 08:18:11     INFO - REFTEST TEST-LOAD | file:///home/worker/workspace/build/tests/reftest/tests/layout/reftests/bugs/291078-1.html | 833 / 1921 (43%)
[task 2017-08-11T08:18:11.808501Z] 08:18:11     INFO - [Child 1121] WARNING: stylo: ServoStyleSets cannot respond to document state changes yet (only matters for chrome documents). See bug 1290285.: file /home/worker/workspace/build/src/layout/base/PresShell.cpp, line 4327
[task 2017-08-11T08:18:11.823625Z] 08:18:11     INFO - ++DOMWINDOW == 45 (0x7ff816c2a800) [pid = 1121] [serial = 2332] [outer = 0x7ff821556800]
[task 2017-08-11T08:18:11.872939Z] 08:18:11     INFO - Assertion failure: f->IsInlineFrame() (Must only have inline frames between us and the first-line frame), at /home/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:1450
[task 2017-08-11T08:18:32.650380Z] 08:18:32     INFO - #01: nsCSSFrameConstructor::CheckForFirstLineInsertion [layout/base/nsCSSFrameConstructor.cpp:11780]
[task 2017-08-11T08:18:32.652344Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.656060Z] 08:18:32     INFO - #02: nsCSSFrameConstructor::ContentAppended [layout/base/nsCSSFrameConstructor.cpp:7915]
[task 2017-08-11T08:18:32.658091Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.659334Z] 08:18:32     INFO - #03: mozilla::PresShell::ContentAppended [layout/base/PresShell.cpp:4439]
[task 2017-08-11T08:18:32.660817Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.661973Z] 08:18:32     INFO - #04: nsNodeUtils::ContentAppended [dom/base/nsNodeUtils.cpp:167]
[task 2017-08-11T08:18:32.662907Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.664076Z] 08:18:32     INFO - #05: nsINode::doInsertChildAt [dom/base/nsINode.cpp:1636]
[task 2017-08-11T08:18:32.665018Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.666253Z] 08:18:32     INFO - #06: nsINode::ReplaceOrInsertBefore [dom/bindings/ErrorResult.h:376]
[task 2017-08-11T08:18:32.667184Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.668333Z] 08:18:32     INFO - #07: mozilla::InsertNodeTransaction::DoTransaction [editor/libeditor/InsertNodeTransaction.cpp:69]
[task 2017-08-11T08:18:32.669296Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.670464Z] 08:18:32     INFO - #08: mozilla::EditorBase::DoTransaction [editor/libeditor/EditorBase.cpp:760]
[task 2017-08-11T08:18:32.671416Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.672536Z] 08:18:32     INFO - #09: mozilla::EditorBase::InsertNode [editor/libeditor/EditorBase.cpp:1515]
[task 2017-08-11T08:18:32.673437Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.674636Z] 08:18:32     INFO - #10: mozilla::TextEditRules::CreateBogusNodeIfNeeded [editor/libeditor/TextEditRules.cpp:1471]
[task 2017-08-11T08:18:32.675615Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.676806Z] 08:18:32     INFO - #11: mozilla::TextEditRules::Init [editor/libeditor/TextEditRules.cpp:131]
[task 2017-08-11T08:18:32.677731Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.678927Z] 08:18:32     INFO - #12: mozilla::TextEditor::InitRules [editor/libeditor/TextEditor.cpp:323]
[task 2017-08-11T08:18:32.679845Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.680962Z] 08:18:32     INFO - #13: mozilla::TextEditor::EndEditorInit [editor/libeditor/TextEditor.cpp:205]
[task 2017-08-11T08:18:32.681951Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.683126Z] 08:18:32     INFO - #14: mozilla::AutoEditInitRulesTrigger::~AutoEditInitRulesTrigger [editor/libeditor/TextEditUtils.cpp:109]
[task 2017-08-11T08:18:32.684002Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.685189Z] 08:18:32     INFO - #15: mozilla::TextEditor::Init [editor/libeditor/TextEditor.cpp:139]
[task 2017-08-11T08:18:32.686079Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.687336Z] 08:18:32     INFO - #16: nsTextEditorState::PrepareEditor [dom/html/nsTextEditorState.cpp:1503]
[task 2017-08-11T08:18:32.688336Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.689573Z] 08:18:32     INFO - #17: nsTextControlFrame::EnsureEditorInitialized [layout/forms/nsTextControlFrame.cpp:290]
[task 2017-08-11T08:18:32.691577Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.692831Z] 08:18:32     INFO - #18: nsTextControlFrame::EditorInitializer::Run [layout/forms/nsTextControlFrame.cpp:1352]
[task 2017-08-11T08:18:32.693802Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.695055Z] 08:18:32     INFO - #19: nsContentUtils::RemoveScriptBlocker [xpcom/base/nsCOMPtr.h:600]
[task 2017-08-11T08:18:32.695952Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.697371Z] 08:18:32     INFO - #20: nsAutoScriptBlocker::~nsAutoScriptBlocker [dom/base/nsContentUtils.h:3372]
[task 2017-08-11T08:18:32.698404Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.699626Z] 08:18:32     INFO - #21: mozilla::PresShell::DoFlushPendingNotifications [layout/base/PresShell.cpp:4207]
[task 2017-08-11T08:18:32.700506Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.701715Z] 08:18:32     INFO - #22: mozilla::PresShell::DoFlushPendingNotifications [layout/base/PresShell.cpp:4077]
[task 2017-08-11T08:18:32.702590Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.703778Z] 08:18:32     INFO - #23: nsDocument::FlushPendingNotifications [dom/base/nsDocument.h:1441]
[task 2017-08-11T08:18:32.704721Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.705874Z] 08:18:32     INFO - #24: nsDocLoader::DocLoaderIsEmpty [uriloader/base/nsDocLoader.cpp:704]
[task 2017-08-11T08:18:32.706788Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.707911Z] 08:18:32     INFO - #25: nsDocLoader::OnStopRequest [uriloader/base/nsDocLoader.cpp:632]
[task 2017-08-11T08:18:32.708876Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.710036Z] 08:18:32     INFO - #26: mozilla::net::nsLoadGroup::RemoveRequest [netwerk/base/nsLoadGroup.cpp:631]
[task 2017-08-11T08:18:32.710967Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.712098Z] 08:18:32     INFO - #27: nsDocument::DoUnblockOnload [dom/base/nsDocument.cpp:8992]
[task 2017-08-11T08:18:32.712958Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.714232Z] 08:18:32     INFO - #28: nsDocument::UnblockOnload [dom/base/nsDocument.cpp:8913]
[task 2017-08-11T08:18:32.715311Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.716598Z] 08:18:32     INFO - #29: nsDocument::DispatchContentLoadedEvents [dom/base/nsDocument.cpp:5352]
[task 2017-08-11T08:18:32.717174Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.718094Z] 08:18:32     INFO - #30: mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0u>::Run [xpcom/threads/nsThreadUtils.h:1176]
[task 2017-08-11T08:18:32.718561Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.719159Z] 08:18:32     INFO - #31: mozilla::SchedulerGroup::Runnable::Run [xpcom/threads/SchedulerGroup.cpp:387]
[task 2017-08-11T08:18:32.719730Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.720324Z] 08:18:32     INFO - #32: nsThread::ProcessNextEvent [mfbt/ScopeExit.h:111]
[task 2017-08-11T08:18:32.720897Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.721471Z] 08:18:32     INFO - #33: NS_ProcessNextEvent [xpcom/threads/nsThreadUtils.cpp:480]
[task 2017-08-11T08:18:32.721568Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.722129Z] 08:18:32     INFO - #34: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:98]
[task 2017-08-11T08:18:32.722667Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.723255Z] 08:18:32     INFO - #35: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:327]
[task 2017-08-11T08:18:32.723808Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.724380Z] 08:18:32     INFO - #36: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:299]
[task 2017-08-11T08:18:32.724923Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.725500Z] 08:18:32     INFO - #37: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:160]
[task 2017-08-11T08:18:32.726023Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.726602Z] 08:18:32     INFO - #38: XRE_RunAppShell [toolkit/xre/nsEmbedFunctions.cpp:882]
[task 2017-08-11T08:18:32.727189Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.727727Z] 08:18:32     INFO - #39: mozilla::ipc::MessagePumpForChildProcess::Run [ipc/glue/MessagePump.cpp:270]
[task 2017-08-11T08:18:32.727784Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.728353Z] 08:18:32     INFO - #40: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:327]
[task 2017-08-11T08:18:32.728836Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.729425Z] 08:18:32     INFO - #41: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:299]
[task 2017-08-11T08:18:32.729978Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.730562Z] 08:18:32     INFO - #42: XRE_InitChildProcess [toolkit/xre/nsEmbedFunctions.cpp:703]
[task 2017-08-11T08:18:32.731102Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.731696Z] 08:18:32     INFO - #43: content_process_main [ipc/contentproc/plugin-container.cpp:66]
[task 2017-08-11T08:18:32.732277Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.732524Z] 08:18:32     INFO - #44: main [browser/app/nsBrowserApp.cpp:288]
[task 2017-08-11T08:18:32.733414Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.734119Z] 08:18:32     INFO - #45: libc.so.6 + 0x20830
[task 2017-08-11T08:18:32.734399Z] 08:18:32     INFO - 
[task 2017-08-11T08:18:32.735346Z] 08:18:32     INFO - #46: _start
Flags: needinfo?(bzbarsky)

    
    

    
  
That assert is simply no longer valid and needs to be removed.

    
    

    
      
  

    
  
Blocks: 1388400

    
    

    
  
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1058b3aaa601
Fix insertions under a ::first-line in stylo.  r=heycam

    
  
Flags: needinfo?(bzbarsky)

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/1058b3aaa601
Status: ASSIGNED → RESOLVED
Closed: 7 years ago

          status-firefox57:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Depends on: 1425959

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: