Closed
Bug 653930
Opened 13 years ago
Closed 13 years ago
Setting marginWidth and marginHeight on an iframe should trigger a reflow
Categories
(Core :: Layout: Images, Video, and HTML Frames, defect)
Core
Layout: Images, Video, and HTML Frames
Tracking
()
RESOLVED
FIXED
mozilla6
People
(Reporter: bholley, Assigned: bholley)
Details
Attachments
(2 files, 1 obsolete file)
2.03 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
3.42 KB,
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
Code that modifies these properties dynamically need to do a little song and dance to get the properties to take effect: http://help.dottoro.com/ljkpxqht.php
The simplest solution here is to implement nsSubDocumentFrame::AttributeChanged (since nsSubDocumentFrame::GetMarginAttributes is where the attributes are handled). Probably both of them should also check that the element in question is an HTML iframe element, though (or perhaps also xul iframe element, if needed).
Comment 2•13 years ago
|
||
Bobby, you want to work on this? If not, I'll steal.
Assignee | ||
Comment 3•13 years ago
|
||
(In reply to comment #2) > Bobby, you want to work on this? If not, I'll steal. I was hoping to get around to it when midterm craziness dies down, but we'll see. So I'll take it for now, and also take the responsibility for pinging you back in a week or two if I decide not to. If there are release timetable reasons why this should happen sooner let me know. ;-)
Assignee: nobody → bobbyholley+bmo
Comment 4•13 years ago
|
||
There is no great rush here. It's not like it ever worked....
Assignee | ||
Comment 5•13 years ago
|
||
I've got the new marginWidth and marginHeight onto nsFrameLoader::mDocShell, but I'm having trouble getting it to take effect. What should I be doing to trigger the restyle here? nsIPresShell::StyleChangeReflow() doesn't seem to be cutting it.
Status: NEW → ASSIGNED
Comment 6•13 years ago
|
||
You probably want to call RebuildAllStyleData() on the prescontext (and not do any manual reflowing; the style data rebuild should do the trick).
I don't think that'll work, since I think the marginwidth and marginheight attributes never actually end up in style data. (Maybe they should, though, but we'd need to figure out as what...)
Comment 8•13 years ago
|
||
> since I think the marginwidth and marginheight attributes never actually end up
> in style data
They do. See BodyRule::MapRuleInfoInto where it grabs those from the docshell.
Should we consider making BodyRule follow the nsIStyleRule contract correctly?
Comment 10•13 years ago
|
||
Sure. I don't think we should make Bobby do that, though....
Assignee | ||
Comment 11•13 years ago
|
||
Attachment #530533 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 12•13 years ago
|
||
Adding tests and fix. Flagging bz for review.
Attachment #530534 -
Flags: review?(bzbarsky)
Comment 13•13 years ago
|
||
Comment on attachment 530534 [details] [diff] [review] Bug 653930 - v1 Just nix the comment in AttributeChanged and don't leave a blank line before the else, please. r=me with that. We should clean up the FrameLoader() thing; I'm not sure it can actually be null here. But good enough for now.
Attachment #530534 -
Flags: review?(bzbarsky) → review+
Comment 14•13 years ago
|
||
Comment on attachment 530533 [details] [diff] [review] Bug 653930 - Tests. v1 r=me
Attachment #530533 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 15•13 years ago
|
||
Addressed bz's comments. Carrying over review.
Attachment #530534 -
Attachment is obsolete: true
Attachment #530667 -
Flags: review+
Assignee | ||
Comment 16•13 years ago
|
||
Pushed to mc: http://hg.mozilla.org/mozilla-central/rev/cf9650c9429a http://hg.mozilla.org/mozilla-central/rev/a7e15c159eec Resolving fixed.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•