Closed
Bug 1172270
Opened 9 years ago
Closed 9 years ago
Initial caching or forcing cache makes websites display furtively without layout
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox38 | --- | unaffected |
firefox38.0.5 | --- | wontfix |
firefox39 | + | disabled |
firefox40 | + | disabled |
firefox41 | + | fixed |
firefox-esr31 | --- | unaffected |
firefox-esr38 | --- | unaffected |
People
(Reporter: epinal99-bugzilla2, Assigned: Gijs)
References
Details
(Keywords: regression)
Attachments
(3 files, 1 obsolete file)
187.06 KB,
image/jpeg
|
Details | |
1.61 KB,
patch
|
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
40 bytes,
text/x-review-board-request
|
Gijs
:
review+
Margaret
:
review+
|
Details |
Issue reported on https://forums.mozfr.org/viewtopic.php?f=5&t=124683 STR: 1) Open http://178.252.19.53/hydra/index.php 2) Force cache (Ctrl+F5) a few times (wait 4-5 secs) Result: The website is displayed without layout during a short time (like no CSS present). You can "mimic" the issue by pressing constantly Ctrl+F5. Various websites have this issue but it's easy to observe with this one. Regresssion range: good=2015-05-07 bad=2015-05-08 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=617dbce26726&tochange=86203ac87a08
Keywords: regression
Comment 1•9 years ago
|
||
[Tracking Requested - why for this release]: Regression Pushlog: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=182674a08067&tochange=584c7f8c6e5b Regressed by: 9d1cf0bd3626 Gijs Kruitbosch — Bug 1160775 - fix reader mode detection to force 1 flush so we don't think the entire page is invisible, r=margaret WORKAROUND Setting reader.parse-on-load.enabled = false helps
Blocks: 1160775
status-firefox38:
--- → unaffected
status-firefox38.0.5:
--- → affected
status-firefox39:
--- → affected
status-firefox40:
--- → affected
status-firefox41:
--- → affected
status-firefox-esr31:
--- → unaffected
status-firefox-esr38:
--- → unaffected
tracking-firefox39:
--- → ?
tracking-firefox40:
--- → ?
tracking-firefox41:
--- → ?
Flags: needinfo?(gijskruitbosch+bugs)
Comment 2•9 years ago
|
||
Another STR 1. Open http://178.252.19.53/hydra/index.php 2. Close other tab if any 3. Exit Browser and restart 4. Clear cache (Ctrl+Shift+Del > Everything > check Cokies, Cache > Clear Now) 5. Restore Previous Session
Updated•9 years ago
|
Assignee | ||
Comment 3•9 years ago
|
||
Sylvestre/Liz, I'd like to back out the offending patch from beta/aurora. I'll work on a proper fix on trunk. That may need to go back up to either, depending on how soon I have it, but for now, I'd like to back this out to minimize where this is broken.
Flags: needinfo?(sledru)
Flags: needinfo?(lhenry)
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8616709 -
Flags: approval-mozilla-beta?
Attachment #8616709 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 4•9 years ago
|
||
No point doing a dot release for 38.0.5 for this with 39 just around the corner.
Comment 5•9 years ago
|
||
Comment on attachment 8616709 [details] [diff] [review] Backout Please uplift to aurora and beta (backs out stuff from bug 1160775).
Flags: needinfo?(lhenry)
Attachment #8616709 -
Flags: approval-mozilla-beta?
Attachment #8616709 -
Flags: approval-mozilla-beta+
Attachment #8616709 -
Flags: approval-mozilla-aurora?
Attachment #8616709 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 6•9 years ago
|
||
remote: https://hg.mozilla.org/releases/mozilla-beta/rev/62f75a6439dd
Comment 7•9 years ago
|
||
Liz, thanks for handling that. I have been also affected myself (mostly on coverity)
Flags: needinfo?(sledru)
Assignee | ||
Comment 8•9 years ago
|
||
remote: https://hg.mozilla.org/releases/mozilla-aurora/rev/1ce864d841db
Assignee | ||
Comment 9•9 years ago
|
||
Olli and Margaret, can you check this *carefully* for inadvertent bugs relating to how all these events are put together, and paints/flushes etc.? I *think* it should work like this, but then, I thought that last time, too.
Attachment #8616935 -
Flags: review?(margaret.leibovic)
Attachment #8616935 -
Flags: review?(bugs)
Comment 10•9 years ago
|
||
Comment on attachment 8616935 [details] [diff] [review] mozafterpaint.patch this should be better yes, and as far as I see, should work well with background tabs too. (not that I quite understand toolkit/components/reader/ReaderMode.jsm code.) It is a bit odd to not check the target of MozAfterPaint, but that shouldn't actually matter here.
Attachment #8616935 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment 11•9 years ago
|
||
Comment on attachment 8616935 [details] [diff] [review] mozafterpaint.patch Review of attachment 8616935 [details] [diff] [review]: ----------------------------------------------------------------- Looks sane to me, although it looks like you're missing the test files. Feel free to flag for re-review when you add the tests (I promise I will be faster!). ::: browser/base/content/tab-content.js @@ +360,5 @@ > content.document.mozSyntheticDocument) { > return; > } > + > + this.scheduleReadabilityCheckPostPaint(forceNonArticle); Will we always expect to receive a MozAfterPaint after calling updateReaderButton? Are we baking in an assumption here about the fact that this helper method is only called after events that are trigger the page content to change? I think that's an okay assumption, since the button should only need to change if the page contents have changed, but we should document that assumption in a comment if that's the case. @@ +386,5 @@ > // Only send updates when there are articles; there's no point updating with > // |false| all the time. > if (ReaderMode.isProbablyReaderable(content.document)) { > sendAsyncMessage("Reader:UpdateReaderButton", { isArticle: true }); > + } else if (this.forceNonArticle) { You passed `forceNonArticle` as a parameter, so no need for the `this` (probably left over from some previous iteration of the patch). ::: browser/base/content/test/general/browser.ini @@ +479,5 @@ > support-files = > readerModeArticle.html > +[browser_readerMode_invisible_nodes.js] > +support-files = > + readerModeArticleHiddenNodes.html Did you forget to hg add these?
Attachment #8616935 -
Flags: review?(margaret.leibovic) → review+
Assignee | ||
Comment 12•9 years ago
|
||
Bug 1172270 - don't cause extra flushes for reader mode, r?margaret,smaug
Attachment #8621607 -
Flags: review?(margaret.leibovic)
Attachment #8621607 -
Flags: review?(bugs)
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8621607 [details] MozReview Request: Bug 1172270 - don't cause extra flushes for reader mode, r?margaret,smaug Carrying over r=smaug
Attachment #8621607 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•9 years ago
|
Attachment #8616935 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8621607 -
Flags: review?(margaret.leibovic) → review+
Comment 14•9 years ago
|
||
Comment on attachment 8621607 [details] MozReview Request: Bug 1172270 - don't cause extra flushes for reader mode, r?margaret,smaug https://reviewboard.mozilla.org/r/11009/#review9623 Ship It!
Comment 16•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fd59f91fb8c9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•